On 08/24/2011 06:20 PM, Chia-I Wu wrote:
> On Thu, Aug 25, 2011 at 8:58 AM, Chad Versace <c...@chad-versace.us> wrote:
> Comments below.
>
> On 08/23/2011 08:10 PM, Chia-I Wu wrote:
>>>> Add platform_android.c that supports _EGL_PLAFORM_ANDROID. It works
>>>> with drm_gralloc, where back buffers of windows are backed by GEM
>>>> objects.
>>>>
>>>> In Android a native window has a queue of back buffers allocated by the
>>>> server, through drm_gralloc. For each frame, EGL needs to
>>>>
>>>> dequeue the next back buffer
>>>> render to the buffer
>>>> enqueue the buffer
>>>>
>>>> After enqueuing, the buffer is no longer valid to EGL. A window has no
>>>> depth buffer or other aux buffers. They need to be allocated locally by
>>>> EGL.
>>>> ---
>>>> src/egl/drivers/dri2/egl_dri2.c | 89 +++++
>>>> src/egl/drivers/dri2/egl_dri2.h | 18 +
>>>> src/egl/drivers/dri2/platform_android.c | 588
>>>> +++++++++++++++++++++++++++++++
>>>> 3 files changed, 695 insertions(+), 0 deletions(-)
>>>> create mode 100644 src/egl/drivers/dri2/platform_android.c
>>>>
>>>> diff --git a/src/egl/drivers/dri2/egl_dri2.c
>>>> b/src/egl/drivers/dri2/egl_dri2.c
>>>> index ba728a1..33c2330 100644
>>>> --- a/src/egl/drivers/dri2/egl_dri2.c
>>>> +++ b/src/egl/drivers/dri2/egl_dri2.c
>
> [snip]
>
>>>> +static _EGLImage *
>>>> +dri2_create_image_android_native_buffer(_EGLDisplay *disp,
>>>> + EGLClientBuffer buffer)
>
> To reiterate Benjamin, I think dri2_create_image_android_native_buffer should
> go
> in android_platform.c.
>
> [snip]
>
>>>> diff --git a/src/egl/drivers/dri2/platform_android.c
>>>> b/src/egl/drivers/dri2/platform_android.c
>>>> new file mode 100644
>>>> index 0000000..d0de94c
>>>> --- /dev/null
>>>> +++ b/src/egl/drivers/dri2/platform_android.c
>
> [snip]
>
>>>> +static _EGLSurface *
>>>> +droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>>>> + _EGLConfig *conf, EGLNativeWindowType window,
>>>> + const EGLint *attrib_list)
>>>> +{
>>>> + struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
>>>> + struct dri2_egl_config *dri2_conf = dri2_egl_config(conf);
>>>> + struct dri2_egl_surface *dri2_surf;
>>>> + int format;
>>>> +
>>>> + (void) drv;
>>>> +
>>>> + if (!window || window->common.magic != ANDROID_NATIVE_WINDOW_MAGIC) {
>>>> + _eglError(EGL_BAD_NATIVE_WINDOW, "droid_create_surface");
>>>> + return NULL;
>>>> + }
>>>> + if (window->query(window, NATIVE_WINDOW_FORMAT, &format)) {
>>>> + _eglError(EGL_BAD_NATIVE_WINDOW, "droid_create_surface");
>>>> + return NULL;
>>>> + }
>>>> + if (format != dri2_conf->base.NativeVisualID) {
>>>> + _eglLog(_EGL_WARNING, "Native format mismatch: 0x%x != 0x%x",
>>>> + format, dri2_conf->base.NativeVisualID);
>>>> + }
>>>> +
>>>> + dri2_surf = calloc(1, sizeof *dri2_surf);
>>>> + if (!dri2_surf) {
>>>> + _eglError(EGL_BAD_ALLOC, "droid_create_surface");
>>>> + return NULL;
>>>> + }
>>>> +
>>>> + if (!_eglInitSurface(&dri2_surf->base, disp, type, conf, attrib_list))
>>>> + goto cleanup_surf;
>
> It seems that droid_get_buffers_with_format() only supports single-buffered
> surfaces. So eglCreateWindowSurface should reject requests for which
> EGL_RENDER_BUFFER != EGL_SINGLE_BUFFER.
>
> if (dri2_surf->base.RenderBuffer != EGL_SINGLE_BUFFER)
> goto cleanup_surf;
>> [CC Benjamin]
>> EGL_RENDER_BUFFER is a hint here. It is fine if the native window
>> does not support EGL_SINGLE_BUFFER.
You're right, specifying EGL_RENDER_BUFFER in eglCreateWindowSurface is just
a hint. And since the spec says this about eglQuerySurface,
Querying EGL_RENDER_BUFFER returns the buffer which client API rendering
is requested to use. For a window surface, this is the same attribute value
specified
when the surface was created.
we shouldn't alter the requested value of dri2_surf->base.RenderBuffer.
>> On the other hand, ctx->WindowRenderBuffer should be set. It is
>> queried by eglQueryContext. I think it can be set in
>> dri2_create_context: to EGL_BACK_BUFFER when there is
>> dri_double_config and EGL_SINGLE_BUFFER when there is only
>> dri_single_config. Idea?
I agree that it seems safe to set ctx->WindowRenderBuffer when there is only a
dri_single_config. So, to ensure that the EGLConfig passed to
eglCreateContext does not have a dri_double_config, I think you need
to add this snippet in droid_add_configs_for_visuals:
for (i = 0; visuals[i].format; i++) {
...
for (j = 0; dri2_dpy->driver_configs[j]; j++) {
...
/* Maybe this? */
if (dri2_dpy->driver_configs[j]->modes.doubleBufferMode)
continue;
/* end suggestion */
dri2_conf = dri2_add_config(dpy, dri2_dpy->driver_configs[j],
count + 1, visuals[i].size, EGL_WINDOW_BIT, NULL,
visuals[i].rgba_masks);
Also, the call to dri2_dpy->dri2->createNewDrawable should be passed
dri2_conf->dri_single_config.
But I am hesitant to set it to EGL_BACK_BUFFER when there is a
dri2_double_config. If the driver supports rendering to a single buffered
surface under a context that supports double-buffering, then, if we had set
ctx->WindowRenderBuffer = EGL_BACK_BUFFER, that would force
eglQueryContext(EGL_RENDER_BUFFER) to always return EGL_BACK_BUFFER, even
when rendering to the single buffered surface.
>
> I think the DRI2 surface type needs to be set here too.
>
> dri2_surf->type = DRI2_WINDOW_SURFACE;
>> This field seems to be used by wayland only. For the other places,
>> dri2_surf->Base.Type is used.
>>>> +
>>>> + dri2_surf->dri_drawable =
>>>> + (*dri2_dpy->dri2->createNewDrawable)(dri2_dpy->dri_screen,
>>>> + dri2_conf->dri_double_config,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I think dri2_conf->dri_single_config should be passed here.
>>>> + dri2_surf);
>>>> + if (dri2_surf->dri_drawable == NULL) {
>>>> + _eglError(EGL_BAD_ALLOC, "dri2->createNewDrawable");
>>>> + goto cleanup_surf;
>>>> + }
>>>> +
>>>> + window->common.incRef(&window->common);
>>>> + window->query(window, NATIVE_WINDOW_WIDTH, &dri2_surf->base.Width);
>>>> + window->query(window, NATIVE_WINDOW_HEIGHT, &dri2_surf->base.Height);
>>>> +
>>>> + dri2_surf->window = window;
>>>> +
>>>> + return &dri2_surf->base;
>>>> +
>>>> +cleanup_surf:
>>>> + free(dri2_surf);
>>>> +
>>>> + return NULL;
>>>> +}
>
> [snip]
>
>>>> +#include <xf86drm.h>
>>>> +/* for i915 */
>>>> +#include <i915_drm.h>
>>>> +/* for radeon */
>>>> +#include <radeon_drm.h>
>
> These includes should be moved to the top of the file.
>> Will do and mention that they are only used by droid_get_pci_id.
>>
>
>
>
--
Chad Versace
c...@chad-versace.us
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
