[Mesa-dev] [PATCH 1/4] radeon/vce: allocate at least 4KB of memory for the feedback buffer

Nicolai Hähnle nicolai.haehnle at amd.com
Thu Sep 29 16:59:22 UTC 2016


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