What special memory placement requirements are those, and how are they expressed?
Uff, where should I start?

The UVD hardware can only access the GPU address space through two special apertures. But most of the time you have to deal with more than two buffers at the same time.

So to make all buffers accessible for a command submission the kernel needs to be able to move them around individually, while still taking care to not make buffers from the command submissions which are currently in flight inaccessible.

If multiple message/feedback/bitstream/context buffers (or even DPB, but I think those are to large) are sub-allocated then the kernel can't move them around individually and you will run into a bunch of problems when you increase the numbers of streams handled at the same time.

If they are captured properly in the alignment requirements and flags then the approach should work.
Not even remotely. Userspace isn't aware of those requirements because it isn't aware of the physical placement of the buffers. That is all handled inside the kernel.

I suggest that you just add a flag to exclude UVD buffers from being sub-allocated.

For VCE the requirements are fortunately not so problematic. I suggest that just increase the buffer size to 4096.

Regards,
Christian.

Am 29.09.2016 um 18:59 schrieb Nicolai Hähnle:
On 29.09.2016 18:52, Christian König wrote:
NAK to the whole approach.

VCE feedback buffers are completely separated to UVD or other MM
feedback buffers, so they shouldn't be allocated in radeon_video.c

I can rename the function to rvce_create_feedback_buffer and move it to radeon_vce.c, that shouldn't be a problem.

Additional to that older UVD message, feedback and bitstream buffers
have special memory placement requirements. So you clearly shouldn't
allocate those from the sub allocator.

What special memory placement requirements are those, and how are they expressed?

If they are captured properly in the alignment requirements and flags then the approach should work.

Nicolai


Sorry that I didn't objected earlier, but I wasn't aware of this change
till now.

Regards,
Christian.

Am 29.09.2016 um 18:35 schrieb Nicolai Hähnle:
From: Nicolai Hähnle <nicolai.haeh...@amd.com>

The kernel's CS checker requires it. This fixes a regression
introduced by
the buffer sub-allocation.

Cc: Christian König <christian.koe...@amd.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97976
---
  src/gallium/drivers/radeon/radeon_vce.c   |  6 +++---
  src/gallium/drivers/radeon/radeon_video.c | 12 ++++++++++++
  src/gallium/drivers/radeon/radeon_video.h |  3 +++
  3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/radeon/radeon_vce.c
b/src/gallium/drivers/radeon/radeon_vce.c
index 10c5a78..dd4c367 100644
--- a/src/gallium/drivers/radeon/radeon_vce.c
+++ b/src/gallium/drivers/radeon/radeon_vce.c
@@ -232,21 +232,21 @@ void rvce_frame_offset(struct rvce_encoder *enc,
struct rvce_cpb_slot *slot,
  }
    /**
   * destroy this video encoder
   */
  static void rvce_destroy(struct pipe_video_codec *encoder)
  {
      struct rvce_encoder *enc = (struct rvce_encoder*)encoder;
      if (enc->stream_handle) {
          struct rvid_buffer fb;
-        rvid_create_buffer(enc->screen, &fb, 512, PIPE_USAGE_STAGING);
+        rvid_create_feedback_buffer(enc->screen, &fb);
          enc->fb = &fb;
          enc->session(enc);
          enc->feedback(enc);
          enc->destroy(enc);
          flush(enc);
          rvid_destroy_buffer(&fb);
      }
      rvid_destroy_buffer(&enc->cpb);
      enc->ws->cs_destroy(enc->cs);
      FREE(enc->cpb_array);
@@ -275,21 +275,21 @@ static void rvce_begin_frame(struct
pipe_video_codec *encoder,
        if (pic->picture_type == PIPE_H264_ENC_PICTURE_TYPE_IDR)
          reset_cpb(enc);
      else if (pic->picture_type == PIPE_H264_ENC_PICTURE_TYPE_P ||
               pic->picture_type == PIPE_H264_ENC_PICTURE_TYPE_B)
          sort_cpb(enc);

      if (!enc->stream_handle) {
          struct rvid_buffer fb;
          enc->stream_handle = rvid_alloc_stream_handle();
-        rvid_create_buffer(enc->screen, &fb, 512, PIPE_USAGE_STAGING);
+        rvid_create_feedback_buffer(enc->screen, &fb);
          enc->fb = &fb;
          enc->session(enc);
          enc->create(enc);
          enc->config(enc);
          enc->feedback(enc);
          flush(enc);
          //dump_feedback(enc, &fb);
          rvid_destroy_buffer(&fb);
          need_rate_control = false;
      }
@@ -304,21 +304,21 @@ static void rvce_begin_frame(struct
pipe_video_codec *encoder,
  static void rvce_encode_bitstream(struct pipe_video_codec *encoder,
                    struct pipe_video_buffer *source,
                    struct pipe_resource *destination,
                    void **fb)
  {
      struct rvce_encoder *enc = (struct rvce_encoder*)encoder;
      enc->get_buffer(destination, &enc->bs_handle, NULL);
      enc->bs_size = destination->width0;
        *fb = enc->fb = CALLOC_STRUCT(rvid_buffer);
-    if (!rvid_create_buffer(enc->screen, enc->fb, 512,
PIPE_USAGE_STAGING)) {
+    if (!rvid_create_feedback_buffer(enc->screen, enc->fb)) {
          RVID_ERR("Can't create feedback buffer.\n");
          return;
      }
      if (!radeon_emitted(enc->cs, 0))
          enc->session(enc);
      enc->encode(enc);
      enc->feedback(enc);
  }
    static void rvce_end_frame(struct pipe_video_codec *encoder,
diff --git a/src/gallium/drivers/radeon/radeon_video.c
b/src/gallium/drivers/radeon/radeon_video.c
index d7c5a16..f60ae05 100644
--- a/src/gallium/drivers/radeon/radeon_video.c
+++ b/src/gallium/drivers/radeon/radeon_video.c
@@ -65,20 +65,32 @@ bool rvid_create_buffer(struct pipe_screen
*screen, struct rvid_buffer *buffer,
              unsigned size, unsigned usage)
  {
      memset(buffer, 0, sizeof(*buffer));
      buffer->usage = usage;
      buffer->res = (struct r600_resource *)
          pipe_buffer_create(screen, PIPE_BIND_CUSTOM, usage, size);
        return buffer->res != NULL;
  }
  +bool rvid_create_feedback_buffer(struct pipe_screen *screen,
+                 struct rvid_buffer *buffer)
+{
+    /* The kernel's CS checker asks for at least 4KB space.
+     *
+     * TODO If we update the kernel checker to be satisfied with less,
+ * we could save some memory here (since the sub-allocator could be
+     * used).
+     */
+ return rvid_create_buffer(screen, buffer, 4096, PIPE_USAGE_STAGING);
+}
+
  /* destroy a buffer */
  void rvid_destroy_buffer(struct rvid_buffer *buffer)
  {
      r600_resource_reference(&buffer->res, NULL);
  }
    /* reallocate a buffer, preserving its content */
  bool rvid_resize_buffer(struct pipe_screen *screen, struct
radeon_winsys_cs *cs,
              struct rvid_buffer *new_buf, unsigned new_size)
  {
diff --git a/src/gallium/drivers/radeon/radeon_video.h
b/src/gallium/drivers/radeon/radeon_video.h
index 39305b4..76c5f30 100644
--- a/src/gallium/drivers/radeon/radeon_video.h
+++ b/src/gallium/drivers/radeon/radeon_video.h
@@ -47,20 +47,23 @@ struct rvid_buffer
      struct r600_resource    *res;
  };
    /* generate an stream handle */
  unsigned rvid_alloc_stream_handle(void);
    /* create a buffer in the winsys */
  bool rvid_create_buffer(struct pipe_screen *screen, struct
rvid_buffer *buffer,
              unsigned size, unsigned usage);
  +bool rvid_create_feedback_buffer(struct pipe_screen *screen,
+                 struct rvid_buffer *buffer);
+
  /* destroy a buffer */
  void rvid_destroy_buffer(struct rvid_buffer *buffer);
    /* reallocate a buffer, preserving its content */
  bool rvid_resize_buffer(struct pipe_screen *screen, struct
radeon_winsys_cs *cs,
              struct rvid_buffer *new_buf, unsigned new_size);
    /* clear the buffer with zeros */
  void rvid_clear_buffer(struct pipe_context *context, struct
rvid_buffer* buffer);




_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to