1. Patchew
  2. QEMU
  3. [PATCH v4 0/7] ui/spice: Enable gl=on option for non-local or remote clients
  4. View patch

[PATCH v4 5/7] ui/console-gl: Add a helper to create a texture with linear memory layout

Vivek Kasireddy posted 7 patches 7 months, 1 week ago
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>
There is a newer version of this series
[PATCH v4 5/7] ui/console-gl: Add a helper to create a texture with linear memory layout
Posted by Vivek Kasireddy 7 months, 1 week ago 
There are cases where we do not want the memory layout of a texture to
be tiled as the component processing the texture would not know how to
de-tile either via software or hardware. Therefore, ensuring that the
memory backing the texture has a linear layout is absolutely necessary
in these situations.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Frediano Ziglio <freddy77@gmail.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 include/ui/console.h |  2 ++
 ui/console-gl.c      | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/include/ui/console.h b/include/ui/console.h
index 46b3128185..5cfa6ae215 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -422,6 +422,8 @@ bool console_gl_check_format(DisplayChangeListener *dcl,
                              pixman_format_code_t format);
 void surface_gl_create_texture(QemuGLShader *gls,
                                DisplaySurface *surface);
+bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
+                                       int fd, GLuint *texture);
 void surface_gl_update_texture(QemuGLShader *gls,
                                DisplaySurface *surface,
                                int x, int y, int w, int h);
diff --git a/ui/console-gl.c b/ui/console-gl.c
index 103b954017..97f7989651 100644
--- a/ui/console-gl.c
+++ b/ui/console-gl.c
@@ -25,6 +25,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "ui/console.h"
 #include "ui/shader.h"
 
@@ -96,6 +97,37 @@ void surface_gl_create_texture(QemuGLShader *gls,
     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
 }
 
+bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
+                                       int fd, GLuint *texture)
+{
+    unsigned long size = surface_stride(surface) * surface_height(surface);
+    GLenum err = glGetError();
+    GLuint mem_obj;
+
+    if (!epoxy_has_gl_extension("GL_EXT_memory_object") ||
+        !epoxy_has_gl_extension("GL_EXT_memory_object_fd")) {
+        return false;
+    }
+
+#ifdef GL_EXT_memory_object_fd
+    glCreateMemoryObjectsEXT(1, &mem_obj);
+    glImportMemoryFdEXT(mem_obj, size, GL_HANDLE_TYPE_OPAQUE_FD_EXT, fd);
+
+    err = glGetError();
+    if (err != GL_NO_ERROR) {
+        error_report("spice: cannot import memory object from fd");
+        return false;
+    }
+
+    glGenTextures(1, texture);
+    glBindTexture(GL_TEXTURE_2D, *texture);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_TILING_EXT, GL_LINEAR_TILING_EXT);
+    glTexStorageMem2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8, surface_width(surface),
+                         surface_height(surface), mem_obj, 0);
+#endif
+    return *texture > 0 && glGetError() == GL_NO_ERROR;
+}
+
 void surface_gl_update_texture(QemuGLShader *gls,
                                DisplaySurface *surface,
                                int x, int y, int w, int h)
-- 
2.49.0
[PATCH v4 5/7] ui/console-gl: Add a helper to create a texture with linear memory layout
Posted by Michael Scherle 6 months, 3 weeks ago 
Great to see this patch making progress.

I've tested it extensively, and unfortunately, I’ve noticed a memory 
leak in surface_gl_create_texture_from_fd(). The memory leak is hard to 
see since the memory is owned by the gpu driver.
On Intel hardware, it's possible to observe the leak using:

cat /sys/module/i915/refcnt
or
xpu-smi ps

In on of my use case—which involves frequent scanout disable/enable 
cycles—the leak is quite apparent. However, in more typical scenarios, 
it might be difficult to catch.

The issue stems from the mem_obj not being deleted after use. I’ve put 
together a minimal modification to address it:



On 15.05.25 04:45, Vivek Kasireddy wrote:
> There are cases where we do not want the memory layout of a texture to
> be tiled as the component processing the texture would not know how to
> de-tile either via software or hardware. Therefore, ensuring that the
> memory backing the texture has a linear layout is absolutely necessary
> in these situations.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Cc: Frediano Ziglio <freddy77@gmail.com>
> Cc: Dongwon Kim <dongwon.kim@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>   include/ui/console.h |  2 ++
>   ui/console-gl.c      | 32 ++++++++++++++++++++++++++++++++
>   2 files changed, 34 insertions(+)
> 
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 46b3128185..5cfa6ae215 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -422,6 +422,8 @@ bool console_gl_check_format(DisplayChangeListener *dcl,
>                                pixman_format_code_t format);
>   void surface_gl_create_texture(QemuGLShader *gls,
>                                  DisplaySurface *surface);
> +bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
> +                                       int fd, GLuint *texture);
>   void surface_gl_update_texture(QemuGLShader *gls,
>                                  DisplaySurface *surface,
>                                  int x, int y, int w, int h);
> diff --git a/ui/console-gl.c b/ui/console-gl.c
> index 103b954017..97f7989651 100644
> --- a/ui/console-gl.c
> +++ b/ui/console-gl.c
> @@ -25,6 +25,7 @@
>    * THE SOFTWARE.
>    */
>   #include "qemu/osdep.h"
> +#include "qemu/error-report.h"
>   #include "ui/console.h"
>   #include "ui/shader.h"
>   
> @@ -96,6 +97,37 @@ void surface_gl_create_texture(QemuGLShader *gls,
>       glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
>   }
>   
> +bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
> +                                       int fd, GLuint *texture)
> +{
> +    unsigned long size = surface_stride(surface) * surface_height(surface);
> +    GLenum err = glGetError();
> +    GLuint mem_obj;

+    GLuint mem_obj = 0;

> +
> +    if (!epoxy_has_gl_extension("GL_EXT_memory_object") ||
> +        !epoxy_has_gl_extension("GL_EXT_memory_object_fd")) {
> +        return false;
> +    }
> +
> +#ifdef GL_EXT_memory_object_fd
> +    glCreateMemoryObjectsEXT(1, &mem_obj);
> +    glImportMemoryFdEXT(mem_obj, size, GL_HANDLE_TYPE_OPAQUE_FD_EXT, fd);
> +
> +    err = glGetError();
> +    if (err != GL_NO_ERROR) {

+          if (mem_obj) {
+              glDeleteMemoryObjectsEXT(1, &mem_obj);
+          }

> +        error_report("spice: cannot import memory object from fd");
> +        return false;
> +    }
> +
> +    glGenTextures(1, texture);
> +    glBindTexture(GL_TEXTURE_2D, *texture);
> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_TILING_EXT, GL_LINEAR_TILING_EXT);
> +    glTexStorageMem2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8, surface_width(surface),
> +                         surface_height(surface), mem_obj, 0);
> +#endif

+    glDeleteMemoryObjectsEXT(1, &mem_obj);

> +    return *texture > 0 && glGetError() == GL_NO_ERROR;
> +}
> +
>   void surface_gl_update_texture(QemuGLShader *gls,
>                                  DisplaySurface *surface,
>                                  int x, int y, int w, int h)



That said, my OpenGL knowledge is somewhat limited, and the 
documentation wasn’t entirely clear to me on whether deleting the memory 
object while the texture is still being used, is always safe. Based on a 
quick look at the iris and llvmpipe implementations, it appears to be 
acceptable.

If that's not the case, an alternative fix could follow this approach 
instead: 
https://gitlab.uni-freiburg.de/opensourcevdi/qemu/-/commit/4ca806871c141089be16af25c1820d3e04f3e27d

Greetings Michael
Re: [PATCH v4 5/7] ui/console-gl: Add a helper to create a texture with linear memory layout
Posted by Michael Scherle 6 months, 3 weeks ago 
Hi all,

I just noticed that Dmitry Osipenko had already pointed out a similar 
issue earlier—so my message was somewhat redundant. Apologies for the 
duplication.

Also, I made a small mistake in the patch I proposed:
The call to glDeleteMemoryObjectsEXT(1, &mem_obj); should be placed 
above the #endif, not after it. Sorry about that oversight!

Best regards,
Michael


On 26.05.25 15:06, Michael Scherle wrote:
> Great to see this patch making progress.
> 
> I've tested it extensively, and unfortunately, I’ve noticed a memory 
> leak in surface_gl_create_texture_from_fd(). The memory leak is hard to 
> see since the memory is owned by the gpu driver.
> On Intel hardware, it's possible to observe the leak using:
> 
> cat /sys/module/i915/refcnt
> or
> xpu-smi ps
> 
> In on of my use case—which involves frequent scanout disable/enable 
> cycles—the leak is quite apparent. However, in more typical scenarios, 
> it might be difficult to catch.
> 
> The issue stems from the mem_obj not being deleted after use. I’ve put 
> together a minimal modification to address it:
> 
> 
> 
> On 15.05.25 04:45, Vivek Kasireddy wrote:
>> There are cases where we do not want the memory layout of a texture to
>> be tiled as the component processing the texture would not know how to
>> de-tile either via software or hardware. Therefore, ensuring that the
>> memory backing the texture has a linear layout is absolutely necessary
>> in these situations.
>>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>> Cc: Frediano Ziglio <freddy77@gmail.com>
>> Cc: Dongwon Kim <dongwon.kim@intel.com>
>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>> ---
>>   include/ui/console.h |  2 ++
>>   ui/console-gl.c      | 32 ++++++++++++++++++++++++++++++++
>>   2 files changed, 34 insertions(+)
>>
>> diff --git a/include/ui/console.h b/include/ui/console.h
>> index 46b3128185..5cfa6ae215 100644
>> --- a/include/ui/console.h
>> +++ b/include/ui/console.h
>> @@ -422,6 +422,8 @@ bool console_gl_check_format(DisplayChangeListener 
>> *dcl,
>>                                pixman_format_code_t format);
>>   void surface_gl_create_texture(QemuGLShader *gls,
>>                                  DisplaySurface *surface);
>> +bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
>> +                                       int fd, GLuint *texture);
>>   void surface_gl_update_texture(QemuGLShader *gls,
>>                                  DisplaySurface *surface,
>>                                  int x, int y, int w, int h);
>> diff --git a/ui/console-gl.c b/ui/console-gl.c
>> index 103b954017..97f7989651 100644
>> --- a/ui/console-gl.c
>> +++ b/ui/console-gl.c
>> @@ -25,6 +25,7 @@
>>    * THE SOFTWARE.
>>    */
>>   #include "qemu/osdep.h"
>> +#include "qemu/error-report.h"
>>   #include "ui/console.h"
>>   #include "ui/shader.h"
>> @@ -96,6 +97,37 @@ void surface_gl_create_texture(QemuGLShader *gls,
>>       glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
>>   }
>> +bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
>> +                                       int fd, GLuint *texture)
>> +{
>> +    unsigned long size = surface_stride(surface) * 
>> surface_height(surface);
>> +    GLenum err = glGetError();
>> +    GLuint mem_obj;
> 
> +    GLuint mem_obj = 0;
> 
>> +
>> +    if (!epoxy_has_gl_extension("GL_EXT_memory_object") ||
>> +        !epoxy_has_gl_extension("GL_EXT_memory_object_fd")) {
>> +        return false;
>> +    }
>> +
>> +#ifdef GL_EXT_memory_object_fd
>> +    glCreateMemoryObjectsEXT(1, &mem_obj);
>> +    glImportMemoryFdEXT(mem_obj, size, GL_HANDLE_TYPE_OPAQUE_FD_EXT, 
>> fd);
>> +
>> +    err = glGetError();
>> +    if (err != GL_NO_ERROR) {
> 
> +          if (mem_obj) {
> +              glDeleteMemoryObjectsEXT(1, &mem_obj);
> +          }
> 
>> +        error_report("spice: cannot import memory object from fd");
>> +        return false;
>> +    }
>> +
>> +    glGenTextures(1, texture);
>> +    glBindTexture(GL_TEXTURE_2D, *texture);
>> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_TILING_EXT, 
>> GL_LINEAR_TILING_EXT);
>> +    glTexStorageMem2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8, 
>> surface_width(surface),
>> +                         surface_height(surface), mem_obj, 0);
>> +#endif
> 
> +    glDeleteMemoryObjectsEXT(1, &mem_obj);
> 
>> +    return *texture > 0 && glGetError() == GL_NO_ERROR;
>> +}
>> +
>>   void surface_gl_update_texture(QemuGLShader *gls,
>>                                  DisplaySurface *surface,
>>                                  int x, int y, int w, int h)
> 
> 
> 
> That said, my OpenGL knowledge is somewhat limited, and the 
> documentation wasn’t entirely clear to me on whether deleting the memory 
> object while the texture is still being used, is always safe. Based on a 
> quick look at the iris and llvmpipe implementations, it appears to be 
> acceptable.
> 
> If that's not the case, an alternative fix could follow this approach 
> instead: https://gitlab.uni-freiburg.de/opensourcevdi/qemu/-/ 
> commit/4ca806871c141089be16af25c1820d3e04f3e27d
> 
> Greetings Michael
RE: [PATCH v4 5/7] ui/console-gl: Add a helper to create a texture with linear memory layout
Posted by Kasireddy, Vivek 6 months, 3 weeks ago 
Hi Michael,

> Subject: Re: [PATCH v4 5/7] ui/console-gl: Add a helper to create a texture
> with linear memory layout
> 
> Hi all,
> 
> I just noticed that Dmitry Osipenko had already pointed out a similar issue
> earlier-so my message was somewhat redundant. Apologies for the
> duplication.
Yeah, looks like you and Dmitry identified the leak independently, almost at the
same time.

> 
> Also, I made a small mistake in the patch I proposed:
> The call to glDeleteMemoryObjectsEXT(1, &mem_obj); should be placed
> above the #endif, not after it. Sorry about that oversight!
Your patch from Open Source VDI repo seems like a better solution, so, I'll add it
to this series and send it out for review in v5. Please add description/commit message
and your signed-off-by line to it. 

Thanks,
Vivek

> 
> Best regards,
> Michael
> 
> 
> On 26.05.25 15:06, Michael Scherle wrote:
> > Great to see this patch making progress.
> >
> > I've tested it extensively, and unfortunately, I've noticed a memory
> > leak in surface_gl_create_texture_from_fd(). The memory leak is hard
> > to see since the memory is owned by the gpu driver.
> > On Intel hardware, it's possible to observe the leak using:
> >
> > cat /sys/module/i915/refcnt
> > or
> > xpu-smi ps
> >
> > In on of my use case-which involves frequent scanout disable/enable
> > cycles-the leak is quite apparent. However, in more typical scenarios,
> > it might be difficult to catch.
> >
> > The issue stems from the mem_obj not being deleted after use. I've put
> > together a minimal modification to address it:
> >
> >
> >
> > On 15.05.25 04:45, Vivek Kasireddy wrote:
> >> There are cases where we do not want the memory layout of a texture
> >> to be tiled as the component processing the texture would not know
> >> how to de-tile either via software or hardware. Therefore, ensuring
> >> that the memory backing the texture has a linear layout is absolutely
> >> necessary in these situations.
> >>
> >> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> >> Cc: Frediano Ziglio <freddy77@gmail.com>
> >> Cc: Dongwon Kim <dongwon.kim@intel.com>
> >> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> >> ---
> >>   include/ui/console.h |  2 ++
> >>   ui/console-gl.c      | 32 ++++++++++++++++++++++++++++++++
> >>   2 files changed, 34 insertions(+)
> >>
> >> diff --git a/include/ui/console.h b/include/ui/console.h index
> >> 46b3128185..5cfa6ae215 100644
> >> --- a/include/ui/console.h
> >> +++ b/include/ui/console.h
> >> @@ -422,6 +422,8 @@ bool
> >> console_gl_check_format(DisplayChangeListener
> >> *dcl,
> >>                                pixman_format_code_t format);
> >>   void surface_gl_create_texture(QemuGLShader *gls,
> >>                                  DisplaySurface *surface);
> >> +bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
> >> +                                       int fd, GLuint *texture);
> >>   void surface_gl_update_texture(QemuGLShader *gls,
> >>                                  DisplaySurface *surface,
> >>                                  int x, int y, int w, int h); diff
> >> --git a/ui/console-gl.c b/ui/console-gl.c index
> >> 103b954017..97f7989651 100644
> >> --- a/ui/console-gl.c
> >> +++ b/ui/console-gl.c
> >> @@ -25,6 +25,7 @@
> >>    * THE SOFTWARE.
> >>    */
> >>   #include "qemu/osdep.h"
> >> +#include "qemu/error-report.h"
> >>   #include "ui/console.h"
> >>   #include "ui/shader.h"
> >> @@ -96,6 +97,37 @@ void surface_gl_create_texture(QemuGLShader *gls,
> >>       glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER,
> >> GL_LINEAR);
> >>   }
> >> +bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
> >> +                                       int fd, GLuint *texture) {
> >> +    unsigned long size = surface_stride(surface) *
> >> surface_height(surface);
> >> +    GLenum err = glGetError();
> >> +    GLuint mem_obj;
> >
> > +    GLuint mem_obj = 0;
> >
> >> +
> >> +    if (!epoxy_has_gl_extension("GL_EXT_memory_object") ||
> >> +        !epoxy_has_gl_extension("GL_EXT_memory_object_fd")) {
> >> +        return false;
> >> +    }
> >> +
> >> +#ifdef GL_EXT_memory_object_fd
> >> +    glCreateMemoryObjectsEXT(1, &mem_obj);
> >> +    glImportMemoryFdEXT(mem_obj, size,
> GL_HANDLE_TYPE_OPAQUE_FD_EXT,
> >> fd);
> >> +
> >> +    err = glGetError();
> >> +    if (err != GL_NO_ERROR) {
> >
> > +          if (mem_obj) {
> > +              glDeleteMemoryObjectsEXT(1, &mem_obj);
> > +          }
> >
> >> +        error_report("spice: cannot import memory object from fd");
> >> +        return false;
> >> +    }
> >> +
> >> +    glGenTextures(1, texture);
> >> +    glBindTexture(GL_TEXTURE_2D, *texture);
> >> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_TILING_EXT,
> >> GL_LINEAR_TILING_EXT);
> >> +    glTexStorageMem2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8,
> >> surface_width(surface),
> >> +                         surface_height(surface), mem_obj, 0);
> >> +#endif
> >
> > +    glDeleteMemoryObjectsEXT(1, &mem_obj);
> >
> >> +    return *texture > 0 && glGetError() == GL_NO_ERROR; }
> >> +
> >>   void surface_gl_update_texture(QemuGLShader *gls,
> >>                                  DisplaySurface *surface,
> >>                                  int x, int y, int w, int h)
> >
> >
> >
> > That said, my OpenGL knowledge is somewhat limited, and the
> > documentation wasn't entirely clear to me on whether deleting the
> > memory object while the texture is still being used, is always safe.
> > Based on a quick look at the iris and llvmpipe implementations, it
> > appears to be acceptable.
> >
> > If that's not the case, an alternative fix could follow this approach
> > instead: https://gitlab.uni-freiburg.de/opensourcevdi/qemu/-/
> > commit/4ca806871c141089be16af25c1820d3e04f3e27d
> >
> > Greetings Michael
RE: [PATCH v4 5/7] ui/console-gl: Add a helper to create a texture with linear memory layout
Posted by Michael Scherle 6 months, 3 weeks ago 
Hi Vivek,

On 27.05.25 06:20, Kasireddy, Vivek wrote:
> Hi Michael,
> 
>> Subject: Re: [PATCH v4 5/7] ui/console-gl: Add a helper to create a texture
>> with linear memory layout
>>
>> Hi all,
>>
>> I just noticed that Dmitry Osipenko had already pointed out a similar issue
>> earlier-so my message was somewhat redundant. Apologies for the
>> duplication.
> Yeah, looks like you and Dmitry identified the leak independently, almost at the
> same time.
> 
>>
>> Also, I made a small mistake in the patch I proposed:
>> The call to glDeleteMemoryObjectsEXT(1, &mem_obj); should be placed
>> above the #endif, not after it. Sorry about that oversight!
> Your patch from Open Source VDI repo seems like a better solution, so, I'll add it
> to this series and send it out for review in v5. Please add description/commit message
> and your signed-off-by line to it.
> 

Thanks! I’ve added it and made a few additional adjustments here:
https://gitlab.uni-freiburg.de/opensourcevdi/qemu/-/commit/859d500761b35cc785fcadd9d554a78712309e88

Feel free to merge it into your patches if you want.

Best regards,
Michael

> Thanks,
> Vivek
> 
>>
>> Best regards,
>> Michael
>>
>>
>> On 26.05.25 15:06, Michael Scherle wrote:
>>> Great to see this patch making progress.
>>>
>>> I've tested it extensively, and unfortunately, I've noticed a memory
>>> leak in surface_gl_create_texture_from_fd(). The memory leak is hard
>>> to see since the memory is owned by the gpu driver.
>>> On Intel hardware, it's possible to observe the leak using:
>>>
>>> cat /sys/module/i915/refcnt
>>> or
>>> xpu-smi ps
>>>
>>> In on of my use case-which involves frequent scanout disable/enable
>>> cycles-the leak is quite apparent. However, in more typical scenarios,
>>> it might be difficult to catch.
>>>
>>> The issue stems from the mem_obj not being deleted after use. I've put
>>> together a minimal modification to address it:
>>>
>>>
>>>
>>> On 15.05.25 04:45, Vivek Kasireddy wrote:
>>>> There are cases where we do not want the memory layout of a texture
>>>> to be tiled as the component processing the texture would not know
>>>> how to de-tile either via software or hardware. Therefore, ensuring
>>>> that the memory backing the texture has a linear layout is absolutely
>>>> necessary in these situations.
>>>>
>>>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>>>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>>> Cc: Frediano Ziglio <freddy77@gmail.com>
>>>> Cc: Dongwon Kim <dongwon.kim@intel.com>
>>>> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>>> ---
>>>>    include/ui/console.h |  2 ++
>>>>    ui/console-gl.c      | 32 ++++++++++++++++++++++++++++++++
>>>>    2 files changed, 34 insertions(+)
>>>>
>>>> diff --git a/include/ui/console.h b/include/ui/console.h index
>>>> 46b3128185..5cfa6ae215 100644
>>>> --- a/include/ui/console.h
>>>> +++ b/include/ui/console.h
>>>> @@ -422,6 +422,8 @@ bool
>>>> console_gl_check_format(DisplayChangeListener
>>>> *dcl,
>>>>                                 pixman_format_code_t format);
>>>>    void surface_gl_create_texture(QemuGLShader *gls,
>>>>                                   DisplaySurface *surface);
>>>> +bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
>>>> +                                       int fd, GLuint *texture);
>>>>    void surface_gl_update_texture(QemuGLShader *gls,
>>>>                                   DisplaySurface *surface,
>>>>                                   int x, int y, int w, int h); diff
>>>> --git a/ui/console-gl.c b/ui/console-gl.c index
>>>> 103b954017..97f7989651 100644
>>>> --- a/ui/console-gl.c
>>>> +++ b/ui/console-gl.c
>>>> @@ -25,6 +25,7 @@
>>>>     * THE SOFTWARE.
>>>>     */
>>>>    #include "qemu/osdep.h"
>>>> +#include "qemu/error-report.h"
>>>>    #include "ui/console.h"
>>>>    #include "ui/shader.h"
>>>> @@ -96,6 +97,37 @@ void surface_gl_create_texture(QemuGLShader *gls,
>>>>        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER,
>>>> GL_LINEAR);
>>>>    }
>>>> +bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
>>>> +                                       int fd, GLuint *texture) {
>>>> +    unsigned long size = surface_stride(surface) *
>>>> surface_height(surface);
>>>> +    GLenum err = glGetError();
>>>> +    GLuint mem_obj;
>>>
>>> +    GLuint mem_obj = 0;
>>>
>>>> +
>>>> +    if (!epoxy_has_gl_extension("GL_EXT_memory_object") ||
>>>> +        !epoxy_has_gl_extension("GL_EXT_memory_object_fd")) {
>>>> +        return false;
>>>> +    }
>>>> +
>>>> +#ifdef GL_EXT_memory_object_fd
>>>> +    glCreateMemoryObjectsEXT(1, &mem_obj);
>>>> +    glImportMemoryFdEXT(mem_obj, size,
>> GL_HANDLE_TYPE_OPAQUE_FD_EXT,
>>>> fd);
>>>> +
>>>> +    err = glGetError();
>>>> +    if (err != GL_NO_ERROR) {
>>>
>>> +          if (mem_obj) {
>>> +              glDeleteMemoryObjectsEXT(1, &mem_obj);
>>> +          }
>>>
>>>> +        error_report("spice: cannot import memory object from fd");
>>>> +        return false;
>>>> +    }
>>>> +
>>>> +    glGenTextures(1, texture);
>>>> +    glBindTexture(GL_TEXTURE_2D, *texture);
>>>> +    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_TILING_EXT,
>>>> GL_LINEAR_TILING_EXT);
>>>> +    glTexStorageMem2DEXT(GL_TEXTURE_2D, 1, GL_RGBA8,
>>>> surface_width(surface),
>>>> +                         surface_height(surface), mem_obj, 0);
>>>> +#endif
>>>
>>> +    glDeleteMemoryObjectsEXT(1, &mem_obj);
>>>
>>>> +    return *texture > 0 && glGetError() == GL_NO_ERROR; }
>>>> +
>>>>    void surface_gl_update_texture(QemuGLShader *gls,
>>>>                                   DisplaySurface *surface,
>>>>                                   int x, int y, int w, int h)
>>>
>>>
>>>
>>> That said, my OpenGL knowledge is somewhat limited, and the
>>> documentation wasn't entirely clear to me on whether deleting the
>>> memory object while the texture is still being used, is always safe.
>>> Based on a quick look at the iris and llvmpipe implementations, it
>>> appears to be acceptable.
>>>
>>> If that's not the case, an alternative fix could follow this approach
>>> instead: https://gitlab.uni-freiburg.de/opensourcevdi/qemu/-/
>>> commit/4ca806871c141089be16af25c1820d3e04f3e27d
>>>
>>> Greetings Michael
>
Re: [PATCH v4 5/7] ui/console-gl: Add a helper to create a texture with linear memory layout
Posted by Dmitry Osipenko 6 months, 4 weeks ago 
On 5/15/25 05:45, Vivek Kasireddy wrote:
> +bool surface_gl_create_texture_from_fd(DisplaySurface *surface,
> +                                       int fd, GLuint *texture)
> +{
> +    unsigned long size = surface_stride(surface) * surface_height(surface);
> +    GLenum err = glGetError();
> +    GLuint mem_obj;
> +
> +    if (!epoxy_has_gl_extension("GL_EXT_memory_object") ||
> +        !epoxy_has_gl_extension("GL_EXT_memory_object_fd")) {
> +        return false;
> +    }
> +
> +#ifdef GL_EXT_memory_object_fd
> +    glCreateMemoryObjectsEXT(1, &mem_obj);
> +    glImportMemoryFdEXT(mem_obj, size, GL_HANDLE_TYPE_OPAQUE_FD_EXT, fd);
> +
> +    err = glGetError();
> +    if (err != GL_NO_ERROR) {
> +        error_report("spice: cannot import memory object from fd");
> +        return false;
> +    }

mem_obj should be destroyed on error and when created texture is released

-- 
Best regards,
Dmitry

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.