<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">On 25.04.25 17:07, Laurent Pinchart
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:20250425150701.GA26250@pendragon.ideasonboard.com">
      <pre wrap="" class="moz-quote-pre">On Fri, Apr 25, 2025 at 09:01:33AM -0400, Nicolas Dufresne wrote:
</pre>
      <blockquote type="cite">
        <pre wrap="" class="moz-quote-pre">Le jeudi 24 avril 2025 à 17:14 +0300, Laurent Pinchart a écrit :
</pre>
        <blockquote type="cite">
          <pre wrap="" class="moz-quote-pre">On Thu, Apr 24, 2025 at 02:51:42PM +0100, Bryan O'Donoghue wrote:
</pre>
          <blockquote type="cite">
            <pre wrap="" class="moz-quote-pre">On 24/04/2025 14:17, Nicolas Dufresne wrote:
</pre>
            <blockquote type="cite">
              <blockquote type="cite">
                <pre wrap="" class="moz-quote-pre">even if the CPU only 'reads' the data ?
</pre>
              </blockquote>
              <pre wrap="" class="moz-quote-pre">Its any CPU write -> GPU that in serious trouble on recent Intel. That
being said, if nothing either flush or invalidate the cache, if you
read before, write with GPU, and read again after that same buffer, the
data you'll read may be from old cache. In practice, GPU don't do
anything in-place, so that is non-issue here.
</pre>
            </blockquote>
            <pre wrap="" class="moz-quote-pre">
Well we have the GPU write to its own frame buffer and currently then 
get a GBM pointer to that surface and memcpy() out into the libcamera 
buffer be that CMA or UDMABuf.

+       gbm_surface_lock_front_buffer(gbm_surface_);
+
+       sync.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_READ;
+       ioctl(bo_fd_, DMA_BUF_IOCTL_SYNC, &sync);
+
+       if (data_len > framesize_) {
+               LOG(GBM, Error) << "Invalid read size " << data_len << " max is " << framesize_;
+               return -EINVAL;
+       }
+
+       memcpy(data, map_, data_len);
+
+       sync.flags = DMA_BUF_SYNC_END | DMA_BUF_SYNC_READ;
+       ioctl(bo_fd_, DMA_BUF_IOCTL_SYNC, &sync);
+
+       gbm_surface_release_buffer(gbm_surface_, gbm_bo_);
+

That I believe should be cache-coherent for both CMA and UDMAbuf across 
architectures.

We had an interesting discussion on how render-to-texture would work - 
if you created the render texture using eglCreateImageKHR.

// render to texture

dmabuf_handle = handle to CMA or UDMABuf buffer.

EGLint attrs[] = {
        EGL_DMA_BUF_PLANE0_FD_EXT, dmabuf_handle
};

mytexture-output = glCreateImageKHR(display, context,
                                     EGL_LINUX_DMA_BUF_EXT,
                                     NULL, attrs);

glBindFramebuffer(GL_FRAMBUFFER, someframebuffer);
glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
                        GL_TEXTURE_2D, mytexture-output);


if dmabuf_handle points to CMA heap - who is responsible to flush the 
cache before the CPU accessess the content of the DMA buf handle..

actually as I type that, I think the answer is that it is always the 
responsibility of the CPU side to manage its own cache so..

glDraw();

sync.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_READ;
ioctl(dmabuf_handle, DMA_BUF_IOCTL_SYNC, &sync);

sync.flags = DMA_BUF_SYNC_END | DMA_BUF_SYNC_READ;
ioctl(dmabuf_handle, DMA_BUF_IOCTL_SYNC, &sync);

something like that ..

</pre>
            <blockquote type="cite">
              <blockquote type="cite">
                <pre wrap="" class="moz-quote-pre">For GPU ISP - the only access the CPU should do is read the data to
generate some statistics. And even that - I would hope in the future
will be turned into operations performed by the GPU ...
</pre>
              </blockquote>
              <pre wrap="" class="moz-quote-pre">So read bayer, process in GPU, output YUV should just work. Thanks for
clarification, running out of CMA is pretty common, defaults CMA
reservation is usually very small. Appart from the buggy drivers,
virtual memory is a much better choice, and this is mostly what this
patch is about here.
</pre>
            </blockquote>
            <pre wrap="" class="moz-quote-pre">
Thinking about this patch.

A user on a system such as imx/hantro is free to configure the system to 
support CMA and UDMABuf.

If you never pass that buffer to the video encoder - where the encoder 
requires phys contig/cma heap memory - then you probably want udmabuf.
</pre>
          </blockquote>
          <pre wrap="" class="moz-quote-pre">
Add the display controller here, it's not just the video encoder.

Have you tried displaying images with cam (using the -D flag) on a
system without an IOMMU, with this patch applied ? Does it still work ?
</pre>
        </blockquote>
        <pre wrap="" class="moz-quote-pre">
In this case glCreateImageKHR() is expected to fail once it discover
that the memory is scattered. In my experience, this part is well
handled.
</pre>
      </blockquote>
      <pre wrap="" class="moz-quote-pre">
It will only fail if the GPU has no IOMMU, right ? What if the GPU has
an IOMMU, but the display controller doesn't ?</pre>
    </blockquote>
    <p><span style="white-space: pre-wrap">This would be the RPi4 case (on the RPi5 the DC has an IOMMU). I've been testing this quite a bit in the context of <a class="moz-txt-link-freetext" href="https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8540">https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8540</a> - using udmabuf with software video decoders in order to reuse dmabuf fastpaths and avoid copies.</span></p>
    <p><span style="white-space: pre-wrap">The DC in question supports DRM_FORMAT_YUV420, which is the native format used by most SW decoders (dav1d, ffpmeg, vpx, openh264, ...) for 8-bit 420 content. So far the case has been handled perfectly as expected - import to GPU (including in Wayland compositors) works but KMS offloading fails in the test commit on the RPi4, while succeeding on RPi5.</span></p>
    <p><span style="white-space: pre-wrap">I've also tested several other devices and have yet to hit a case where things blow up because some component requiring CMA tries to import an udmabuf. The concern that it could happen has been rased by RPi devs as well though (<a class="moz-txt-link-freetext" href="https://github.com/raspberrypi/linux/issues/6706#issuecomment-2706283848">https://github.com/raspberrypi/linux/issues/6706#issuecomment-2706283848</a>).</span></p>
    <p><span style="white-space: pre-wrap">---</span></p>
    <p><span style="white-space: pre-wrap">Either way, do we agree that the commit in question should have its own series for further discussion as it's only semi-related to the gpuISP?
</span></p>
    <p><span style="white-space: pre-wrap">
</span></p>
    <blockquote type="cite"
      cite="mid:20250425150701.GA26250@pendragon.ideasonboard.com">
      <pre wrap="" class="moz-quote-pre">
</pre>
      <blockquote type="cite">
        <pre wrap="" class="moz-quote-pre">What's missing sometimes is verification that memory
allocation have happened with CPU cached enabled. Without the mmu,
there is nothing to handle cached pages. Recent Intel graphics happily
succeed, resulting in a funny mess on screen.

I assume the code here is just for example, and that there is failure
and fallback in place.

</pre>
        <blockquote type="cite">
          <blockquote type="cite">
            <pre wrap="" class="moz-quote-pre">The reverse is also true.

Its almost use-case specific. If you want the encoder you need CMA and 
if you just want to say - display the output in cheese you want UDMAbuf.

Probably the right thing to do is to leave CMA heap as default and leave 
it to the system designer to configure the system as they wish.

In my case, switching off the CMA heap or perhaps a libcamera config/env 
variable to specify which to use...
</pre>
          </blockquote>
        </blockquote>
      </blockquote>
      <pre wrap="" class="moz-quote-pre">
</pre>
    </blockquote>
  </body>
</html>