[Mesa-dev] [PATCH] EGL: Add pbuffer support for drm platform

[Liu, Ying2]

Hi, Chad,

Thank you so much for reviewing my patch. I am in vacation and sabbatical right now. I will talk with our customer about your suggestion. I will fix the patch when I come back.

Ying

-----Original Message-----
From: Versace, Chad 
Sent: Thursday, June 18, 2015 7:34 AM
To: Liu, Ying2
Cc: mesa-dev at lists.freedesktop.org; Marek Olšák
Subject: Re: [Mesa-dev] [PATCH] EGL: Add pbuffer support for drm platform

On Thu 11 Jun 2015, Ying Liu wrote:
> Add pbuffer support for drm platform, because some customers are still using this feature.
> 
> Signed-off-by: Ying Liu <ying2.liu at intel.com>
> ---
>  src/egl/drivers/dri2/egl_dri2.c     |  2 +-
>  src/egl/drivers/dri2/platform_drm.c | 18 ++++++++++++++----
>  2 files changed, 15 insertions(+), 5 deletions(-)

Ying, I wrote a simple test for this patch that calls glClear on the pbuffer.
The test crashes deep in the Intel driver. The test code is attached.

Below is the backtrace and crashing line of code. I applied your patch to 6b0378e483ba53359545ac8b774dbdd81c2fab3f.

    file: src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
    450+>      if (irb->mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_NO_MCS)
    451|          clear_type = REP_CLEAR;
    452|

    #0  0x00007ffff4f344db in brw_meta_fast_clear (brw=0x7ffff7f97040, fb=0xdbbe70, buffers=2, partial_clear=false) at brw_meta_fast_clear.c:450
    #1  0x00007ffff4ebf2f2 in brw_clear (ctx=0x7ffff7f97040, mask=2) at brw_clear.c:247
    #2  0x00007ffff4ad0baa in _mesa_Clear (mask=16384) at main/clear.c:224
    #3  0x0000000000400e4c in main () at examples/play-gbm-pbuffer.c:69

The miptree pointer 'mt' is NULL. That suggests that Mesa never allocated buffer storage for the pbuffer.

If you want to continue with this patch, this is what you need to:

    - Fix the crash. I suspect that you need to create a gbm_bo for the pbuffer
      in dri2_drm_create_pbuffer_surface() and bind it to the egl_dri2_surface or
      DRI2Drawable, similar to how platform_x11.c allocates a pixmap for pbuffer
      surfaces.

    - Submit a Piglit test for gbm pbuffers to piglit at lists.freedesktop.org.
      The test should go somewhere under the 'tests/egl' directory. You may
      want to start with my test code, but add color probing to it. After the
      glClear, call glReadPixels and then verify the pixels have the correct color.
      That would a be a very simple test, and I think it's sufficient for this patch.

BUT... I don't understand how this customer could possibly use GBM pbuffers, except very badly. Whatever the customer is doing with pbuffers, there is better way with better performance and less bugs. Does your customer actually render to the pbuffer? Or does your customer create the pbuffer only for the sake of providing a surface to eglMakeCurrent, and then never actually use it?

If the customer is using the pbuffer as a dummy surface for eglMakeCurrent, then there is a better alternative. In nearly all EGL implementations (the closed source ones too), it is legal to call eglMakeCurrent() *without* a surface.
That is, you can do this:
    eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, context);


// Chad's little gbm pbuffer test.
// file: try-gbm-pbuffer.c
//

#define _POSIX_C_SOURCE 200809L

#include <assert.h>
#include <stdbool.h>
#include <stdlib.h>

#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

#include <gbm.h>
#include <GLES2/gl2.h>
#include <EGL/egl.h>
#include <EGL/eglext.h>

int
main(void)
{
    bool ok;

    int fd = open("/dev/dri/renderD128", O_RDWR | O_CLOEXEC);
    assert(fd >= 0);

    struct gbm_device *gbm_dev = gbm_create_device(fd);
    assert(gbm_dev);

    EGLDisplay dpy = eglGetDisplay(gbm_dev);
    assert(dpy);

    EGLint major, minor;
    ok = eglInitialize(dpy, &major, &minor);
    assert(ok);

    EGLint config_attrs[] = {
        EGL_SURFACE_TYPE,               EGL_PBUFFER_BIT,
        EGL_RENDERABLE_TYPE,            EGL_OPENGL_ES2_BIT,

        EGL_BUFFER_SIZE,                32,
        EGL_RED_SIZE,                   8,
        EGL_GREEN_SIZE,                 8,
        EGL_BLUE_SIZE,                  8,
        EGL_ALPHA_SIZE,                 8,

        EGL_NONE,
    };

    EGLConfig config = 0;
    EGLint num_configs = 0;
    eglChooseConfig(dpy, config_attrs, &config, 1, &num_configs);
    assert(num_configs >= 1);
    assert(config);

    EGLSurface surf = eglCreatePbufferSurface(dpy, config, NULL);
    assert(surf);

    EGLint ctx_attrs[] = {
        EGL_CONTEXT_MAJOR_VERSION_KHR, 3,
        EGL_CONTEXT_MINOR_VERSION_KHR, 0,
        EGL_NONE,
    };

    EGLContext ctx = eglCreateContext(dpy, config, NULL, ctx_attrs);
    assert(ctx);

    ok = eglMakeCurrent(dpy, surf, surf, ctx);
    assert(ok);

    glClearColor(1, 0, 0, 1);
    glClear(GL_COLOR_BUFFER_BIT);
    eglSwapBuffers(dpy, surf);
    glFinish();

    return 0;
}