[Mesa-dev] [PATCH] egl/dri2: implement platform_null.

[Haixia Shi]

Hi Emil,

On Thu, Jan 22, 2015 at 4:38 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 22/01/15 22:23, Haixia Shi wrote:
>> Hi Emil
>>
>> On Thu, Jan 22, 2015 at 1:36 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>> Hi Haixia Shi,
>>>
>>> On 22/01/15 17:35, Haixia Shi wrote:
>>>> Try the render node first and use it if available. Otherwise fall back to
>>>> normal nodes.
>>>>
>>> What is the use-case for such a platform - I assume it's worth
>>> mentioning in the commit message ?
>>>
>>> No other platform picks the device at random as seen below. Why did you
>>> choose such an approach ? It seems like one can easily shoot themselves
>>> by using it.
>>
>> CC Stephane. The goal here is just to pick the first available node
>> for off-screen rendering only.
>>
> Hmm I'm guessing that using the drm/gbm platform is out of the question
> ? Iirc there has been a bit of love on the gbm topic, and afaiu this
> solution is to be used with minigbm ?

Yes this solutions is to be used with minigbm.

>
> What I'm thinking here is:
> If you're testing a device with provides two or more nodes (be that the
> classic card or the render ones), one cannot guarantee that the kernel
> module for hw#1 will be loaded first. Thus even if one presumes that
> they are working on (testing) hw#1 that may or may not be the case.
>
> Not 100% sure on the module order part, so I could be wrong.

I don't have a good answer for that... any suggestion on how best to
pick the right one?

>
>>>
>>> ...
>>>> diff --git a/src/egl/drivers/dri2/platform_null.c b/src/egl/drivers/dri2/platform_null.c
>>>> new file mode 100644
>>>> index 0000000..4f0b18f
>>>> --- /dev/null
>>>> +++ b/src/egl/drivers/dri2/platform_null.c
>>> ...
>>>> +static const char* node_path_fmt_card = "/dev/dri/card%d";
>>> You can reuse the DRM_DIR_NAME + DRM_DEV_NAME macros (from xf86drm.h)
>>> for this.
>>>
>>>> +static const char* node_path_fmt_render = "/dev/dri/renderD%d";
>>> There is no macro for the renderD%d, although you can still use
>>> DRM_DIR_NAME for the path.
>>
>> Will update this part shortly.
>>
> I would personally wait for more feedback, rather than going through xx
> revisions. But that's just me being lazy :P
>
>>>
>>>> +
>>>> +EGLBoolean
>>>> +dri2_initialize_null(_EGLDriver *drv, _EGLDisplay *disp)
>>>> +{
>>>> +   struct dri2_egl_display *dri2_dpy;
>>>> +   const char* err;
>>>> +   int i, render_node;
>>>> +   int driver_loaded = 0;
>>>> +
>>>> +   loader_set_logger(_eglLog);
>>>> +
>>>> +   dri2_dpy = calloc(1, sizeof *dri2_dpy);
>>>> +   if (!dri2_dpy)
>>>> +      return _eglError(EGL_BAD_ALLOC, "eglInitialize");
>>>> +
>>>> +   disp->DriverData = (void *) dri2_dpy;
>>>> +
>>>> +   for (render_node = 1; render_node >= 0; --render_node) {
>>>> +      const char* node_path_fmt =
>>>> +            render_node ? node_path_fmt_render : node_path_fmt_card;
>>>> +      const int base = render_node ? 128 : 0;
>>>> +      for (i = 0; i < 16; ++i) {
>>> What was the reason behind choosing 16 here ?
>>
>> It's an arbitrary number.
> My bad it's DRM_MAX_MINOR, as defined in xf86drm.h. Please use the
> symbolic name.
>
>> How about choosing 64 here as it is the
>> limit chosen by drm_stub.c: drm_minor_get_id()?
>>
> We nuked the file with kernel 3.17 :P
>
>>>
>>>> +         char *card_path;
>>>> +         if (asprintf(&card_path, node_path_fmt, base + i) < 0)
>>>> +            continue;
>>>> +
>>>> +         dri2_dpy->fd = open(card_path, O_RDWR);
>>> If you open a normal node (card%d) I believe that you'll need an
>>> authenticate hook in dri2_egl_display_vtbl. Does things work without it
>>> on your system/platform ?
>>
>> You're correct; normal node would require the legacy auth hook, and it
>> would only work without auth if the process is run as root, which is
>> why we're trying render nodes first.
>>
> So you're saying that people without render nodes should run egl(mesa)
> as root ? That does not sound like a wise suggestion imho.
>
> Basically what I'm trying to say is - if you have a fall-back to normal
> nodes, some form of auth ought to be in place.

I see your point. Would it be cleaner if we simply require render node
to be present? The normal node (card%d) and the auth hook is more
trouble than its worth.

>
> Not an expert on the topic
> -Emil