[Mesa-dev] [PATCH 1/4] radeon/vce: allocate at least 4KB of memory for the feedback buffer
Christian König
christian.koenig at amd.com
Thu Sep 29 17:23:35 UTC 2016
> 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.haehnle at amd.com>
>>>
>>> The kernel's CS checker requires it. This fixes a regression
>>> introduced by
>>> the buffer sub-allocation.
>>>
>>> Cc: Christian König <christian.koenig at 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);
>>>
>>
>>
More information about the mesa-dev
mailing list