Hi Emil,
I have added comments for each patch, and applied for branch 12.0. Please find attached patches. Thanks, Sonny ________________________________ From: mesa-dev <mesa-dev-boun...@lists.freedesktop.org> on behalf of Christian König <deathsim...@vodafone.de> Sent: Friday, July 1, 2016 8:07:51 AM To: Emil Velikov Cc: Jiang, Sonny; 12.0; mesa-dev@lists.freedesktop.org Subject: Re: [Mesa-dev] [Mesa-stable] [PATCH] radeon uvd add uvd fw version for amdgpu Am 01.07.2016 um 13:14 schrieb Emil Velikov: > Hi all, > > On 29 June 2016 at 20:20, Christian König <deathsim...@vodafone.de> wrote: >> Am 29.06.2016 um 18:35 schrieb Alex Deucher: >>> On Wed, Jun 29, 2016 at 11:38 AM, Leo Liu <leo....@amd.com> wrote: >>>> From: sonjiang <sonny.ji...@amd.com> >>>> >>>> Signed-off-by: sonjiang <sonny.ji...@amd.com> >>>> Cc: "12.0" <mesa-sta...@lists.freedesktop.org> >>> For the series: >>> Reviewed-by: Alex Deucher <alexander.deuc...@amd.com> >> >> Reviewed-by: Christian König <christian.koe...@amd.com> as well. >> > Here we have three patches, suggesting a bug with absolutely no > information what the issue is and/or why this approach is correct. > > I'm sorry to say this, but as is, this series is not landing in > stable. Sonjiang, being the author of these please reply with a brief > justification why we want those. Before doing so I would strongly > recommend reading this [1] blog post. Well to put a carrot on the front of your stick: I asked what the firmware version patch is all about internally as well when I've seen those patches. So it would have even made our internal review much easier if Sonny added a commit message in the first place. My fault to not requesting that his answer is put as a commit message on the patches. On the other hand this is for Polaris, we had time pressure to get it out of the door and today is a public holiday in Canada. So you probably won't get updated message before Monday. Is that soon enough? Otherwise UVD will be broken on Polaris in the stable branch. Regards, Christian. > > Thanks > Emil > > [1] http://who-t.blogspot.co.uk/2009/12/on-commit-messages.html _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
From 53248a0392b1baf4c1cf67600088557f16d6ec8a Mon Sep 17 00:00:00 2001 From: sonjiang <sonny.ji...@amd.com> Date: Mon, 4 Jul 2016 16:48:37 -0400 Subject: [PATCH] radeon uvd add uvd fw version for amdgpu Because Polaris uvd fw interface changes, the driver need to check fw version to apply right interface. This change is to add uvd fw version. Signed-off-by: sonjiang <sonny.ji...@amd.com> Cc: "12.0" <mesa-sta...@lists.freedesktop.org> Reviewed-by: Alex Deucher <alexander.deuc...@amd.com> Reviewed-by: Christian König <christian.koe...@amd.com> --- src/gallium/drivers/radeon/radeon_winsys.h | 1 + src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/gallium/drivers/radeon/radeon_winsys.h b/src/gallium/drivers/radeon/radeon_winsys.h index 699a9eb..8b16325 100644 --- a/src/gallium/drivers/radeon/radeon_winsys.h +++ b/src/gallium/drivers/radeon/radeon_winsys.h @@ -250,6 +250,7 @@ struct radeon_info { bool gfx_ib_pad_with_type2; boolean has_sdma; boolean has_uvd; + uint32_t uvd_fw_version; uint32_t vce_fw_version; uint32_t vce_harvest_config; uint32_t clock_crystal_freq; diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c index 7016221..38b0376 100644 --- a/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_winsys.c @@ -98,7 +98,7 @@ static boolean do_winsys_init(struct amdgpu_winsys *ws, int fd) struct amdgpu_buffer_size_alignments alignment_info = {}; struct amdgpu_heap_info vram, gtt; struct drm_amdgpu_info_hw_ip dma = {}, uvd = {}, vce = {}; - uint32_t vce_version = 0, vce_feature = 0; + uint32_t vce_version = 0, vce_feature = 0, uvd_version = 0, uvd_feature = 0; int r, i, j; drmDevicePtr devinfo; @@ -151,6 +151,13 @@ static boolean do_winsys_init(struct amdgpu_winsys *ws, int fd) goto fail; } + r = amdgpu_query_firmware_version(ws->dev, AMDGPU_INFO_FW_UVD, 0, 0, + &uvd_version, &uvd_feature); + if (r) { + fprintf(stderr, "amdgpu: amdgpu_query_firmware_version(uvd) failed.\n"); + goto fail; + } + r = amdgpu_query_hw_ip_info(ws->dev, AMDGPU_HW_IP_VCE, 0, &vce); if (r) { fprintf(stderr, "amdgpu: amdgpu_query_hw_ip_info(vce) failed.\n"); @@ -268,6 +275,8 @@ static boolean do_winsys_init(struct amdgpu_winsys *ws, int fd) ws->info.max_se = ws->amdinfo.num_shader_engines; ws->info.max_sh_per_se = ws->amdinfo.num_shader_arrays_per_engine; ws->info.has_uvd = uvd.available_rings != 0; + ws->info.uvd_fw_version = + uvd.available_rings ? uvd_version : 0; ws->info.vce_fw_version = vce.available_rings ? vce_version : 0; ws->info.has_userptr = TRUE; -- 2.7.4
From 6387dd496cca77fb08b7600796156695980bb0b0 Mon Sep 17 00:00:00 2001 From: sonjiang <sonny.ji...@amd.com> Date: Mon, 27 Jun 2016 17:19:01 -0400 Subject: [PATCH 2/3] radeon/uvd: seperate uvd context buffer from DPB Adapt driver for Polairs uvd firmware interface changes. Signed-off-by: sonjiang <sonny.ji...@amd.com> Cc: "12.0" <mesa-sta...@lists.freedesktop.org> Reviewed-by: Alex Deucher <alexander.deuc...@amd.com> Reviewed-by: Christian König <christian.koe...@amd.com> --- src/gallium/drivers/radeon/radeon_uvd.c | 106 +++++++++++++++++++++++++++++--- 1 file changed, 97 insertions(+), 9 deletions(-) diff --git a/src/gallium/drivers/radeon/radeon_uvd.c b/src/gallium/drivers/radeon/radeon_uvd.c index a2d1d2d..1f28b01 100644 --- a/src/gallium/drivers/radeon/radeon_uvd.c +++ b/src/gallium/drivers/radeon/radeon_uvd.c @@ -60,6 +60,8 @@ #define FB_BUFFER_SIZE_TONGA (2048 * 64) #define IT_SCALING_TABLE_SIZE 992 +#define FW_1_66_16 ((1 << 24) | (66 << 16) | (16 << 8)) + /* UVD decoder representation */ struct ruvd_decoder { struct pipe_video_codec base; @@ -208,6 +210,60 @@ static uint32_t profile2stream_type(struct ruvd_decoder *dec, unsigned family) } } +static unsigned calc_ctx_size_h264_perf(struct ruvd_decoder *dec) +{ + unsigned width_in_mb, height_in_mb, ctx_size; + unsigned width = align(dec->base.width, VL_MACROBLOCK_WIDTH); + unsigned height = align(dec->base.height, VL_MACROBLOCK_HEIGHT); + + unsigned max_references = dec->base.max_references + 1; + + // picture width & height in 16 pixel units + width_in_mb = width / VL_MACROBLOCK_WIDTH; + height_in_mb = align(height / VL_MACROBLOCK_HEIGHT, 2); + + if (!dec->use_legacy) { + unsigned fs_in_mb = width_in_mb * height_in_mb; + unsigned num_dpb_buffer; + switch(dec->base.level) { + case 30: + num_dpb_buffer = 8100 / fs_in_mb; + break; + case 31: + num_dpb_buffer = 18000 / fs_in_mb; + break; + case 32: + num_dpb_buffer = 20480 / fs_in_mb; + break; + case 41: + num_dpb_buffer = 32768 / fs_in_mb; + break; + case 42: + num_dpb_buffer = 34816 / fs_in_mb; + break; + case 50: + num_dpb_buffer = 110400 / fs_in_mb; + break; + case 51: + num_dpb_buffer = 184320 / fs_in_mb; + break; + default: + num_dpb_buffer = 184320 / fs_in_mb; + break; + } + num_dpb_buffer++; + max_references = MAX2(MIN2(NUM_H264_REFS, num_dpb_buffer), max_references); + ctx_size = max_references * align(width_in_mb * height_in_mb * 192, 256); + } else { + // the firmware seems to always assume a minimum of ref frames + max_references = MAX2(NUM_H264_REFS, max_references); + // macroblock context buffer + ctx_size = align(width_in_mb * height_in_mb * max_references * 192, 256); + } + + return ctx_size; +} + static unsigned calc_ctx_size_h265_main(struct ruvd_decoder *dec) { unsigned width = align(dec->base.width, VL_MACROBLOCK_WIDTH); @@ -316,17 +372,23 @@ static unsigned calc_dpb_size(struct ruvd_decoder *dec) num_dpb_buffer++; max_references = MAX2(MIN2(NUM_H264_REFS, num_dpb_buffer), max_references); dpb_size = image_size * max_references; - dpb_size += max_references * align(width_in_mb * height_in_mb * 192, alignment); - dpb_size += align(width_in_mb * height_in_mb * 32, alignment); + if ((dec->stream_type != RUVD_CODEC_H264_PERF) || + (((struct r600_common_screen*)dec->screen)->family < CHIP_POLARIS10)) { + dpb_size += max_references * align(width_in_mb * height_in_mb * 192, alignment); + dpb_size += align(width_in_mb * height_in_mb * 32, alignment); + } } else { // the firmware seems to allways assume a minimum of ref frames max_references = MAX2(NUM_H264_REFS, max_references); // reference picture buffer dpb_size = image_size * max_references; - // macroblock context buffer - dpb_size += width_in_mb * height_in_mb * max_references * 192; - // IT surface buffer - dpb_size += width_in_mb * height_in_mb * 32; + if ((dec->stream_type != RUVD_CODEC_H264_PERF) || + (((struct r600_common_screen*)dec->screen)->family < CHIP_POLARIS10)) { + // macroblock context buffer + dpb_size += width_in_mb * height_in_mb * max_references * 192; + // IT surface buffer + dpb_size += width_in_mb * height_in_mb * 32; + } } break; } @@ -877,7 +939,9 @@ static void ruvd_destroy(struct pipe_video_codec *decoder) } rvid_destroy_buffer(&dec->dpb); - if (u_reduce_video_profile(dec->base.profile) == PIPE_VIDEO_FORMAT_HEVC) + if ((u_reduce_video_profile(dec->base.profile) == PIPE_VIDEO_FORMAT_HEVC) || + (dec->stream_type == RUVD_CODEC_H264_PERF && + ((struct r600_common_screen*)dec->screen)->family >= CHIP_POLARIS10)) rvid_destroy_buffer(&dec->ctx); FREE(dec); @@ -1006,6 +1070,10 @@ static void ruvd_end_frame(struct pipe_video_codec *decoder, dec->msg->body.decode.bsd_size = bs_size; dec->msg->body.decode.db_pitch = align(dec->base.width, 16); + if (dec->stream_type == RUVD_CODEC_H264_PERF && + ((struct r600_common_screen*)dec->screen)->family >= CHIP_POLARIS10) + dec->msg->body.decode.dpb_reserved = dec->ctx.res->buf->size; + dt = dec->set_dtb(dec->msg, (struct vl_video_buffer *)target); if (((struct r600_common_screen*)dec->screen)->family >= CHIP_STONEY) dec->msg->body.decode.dt_wa_chroma_top_offset = dec->msg->body.decode.dt_pitch / 2; @@ -1057,7 +1125,9 @@ static void ruvd_end_frame(struct pipe_video_codec *decoder, send_cmd(dec, RUVD_CMD_DPB_BUFFER, dec->dpb.res->buf, 0, RADEON_USAGE_READWRITE, RADEON_DOMAIN_VRAM); - if (u_reduce_video_profile(picture->profile) == PIPE_VIDEO_FORMAT_HEVC) { + if ((u_reduce_video_profile(picture->profile) == PIPE_VIDEO_FORMAT_HEVC) || + (dec->stream_type == RUVD_CODEC_H264_PERF && + ((struct r600_common_screen*)dec->screen)->family >= CHIP_POLARIS10)) { send_cmd(dec, RUVD_CMD_CONTEXT_BUFFER, dec->ctx.res->buf, 0, RADEON_USAGE_READWRITE, RADEON_DOMAIN_VRAM); } @@ -1108,7 +1178,16 @@ struct pipe_video_codec *ruvd_create_decoder(struct pipe_context *context, /* fall through */ case PIPE_VIDEO_FORMAT_MPEG4: + width = align(width, VL_MACROBLOCK_WIDTH); + height = align(height, VL_MACROBLOCK_HEIGHT); + break; case PIPE_VIDEO_FORMAT_MPEG4_AVC: + if ((info.family == CHIP_POLARIS10 || info.family == CHIP_POLARIS11) && + info.uvd_fw_version < FW_1_66_16 ) { + RVID_ERR("POLARIS10/11 firmware version need to be updated.\n"); + return NULL; + } + width = align(width, VL_MACROBLOCK_WIDTH); height = align(height, VL_MACROBLOCK_HEIGHT); break; @@ -1182,6 +1261,15 @@ struct pipe_video_codec *ruvd_create_decoder(struct pipe_context *context, rvid_clear_buffer(context, &dec->dpb); + if (dec->stream_type == RUVD_CODEC_H264_PERF && info.family >= CHIP_POLARIS10) { + unsigned ctx_size = calc_ctx_size_h264_perf(dec); + if (!rvid_create_buffer(dec->screen, &dec->ctx, ctx_size, PIPE_USAGE_DEFAULT)) { + RVID_ERR("Can't allocated context buffer.\n"); + goto error; + } + rvid_clear_buffer(context, &dec->ctx); + } + map_msg_fb_it_buf(dec); dec->msg->size = sizeof(*dec->msg); dec->msg->msg_type = RUVD_MSG_CREATE; @@ -1205,7 +1293,7 @@ error: } rvid_destroy_buffer(&dec->dpb); - if (u_reduce_video_profile(dec->base.profile) == PIPE_VIDEO_FORMAT_HEVC) + if (dec->stream_type == RUVD_CODEC_H264_PERF && info.family >= CHIP_POLARIS10) rvid_destroy_buffer(&dec->ctx); FREE(dec); -- 2.7.4
From c57e350c209a462a541c36ab52b5a4b2233679be Mon Sep 17 00:00:00 2001 From: sonjiang <sonny.ji...@amd.com> Date: Tue, 28 Jun 2016 11:23:41 -0400 Subject: [PATCH 3/3] radeon/uvd: fix a h265 context size bug Fixes a h265 video corruption bug which caused by uvd fw interface changes. Signed-off-by: sonjiang <sonny.ji...@amd.com> Cc: "12.0" <mesa-sta...@lists.freedesktop.org> Reviewed-by: Alex Deucher <alexander.deuc...@amd.com> Reviewed-by: Christian König <christian.koe...@amd.com> --- src/gallium/drivers/radeon/radeon_uvd.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/gallium/drivers/radeon/radeon_uvd.c b/src/gallium/drivers/radeon/radeon_uvd.c index 1f28b01..7d0d2fd 100644 --- a/src/gallium/drivers/radeon/radeon_uvd.c +++ b/src/gallium/drivers/radeon/radeon_uvd.c @@ -1096,6 +1096,9 @@ static void ruvd_end_frame(struct pipe_video_codec *decoder, } rvid_clear_buffer(decoder->context, &dec->ctx); } + + if (dec->ctx.res) + dec->msg->body.decode.dpb_reserved = dec->ctx.res->buf->size; break; case PIPE_VIDEO_FORMAT_VC1: -- 2.7.4
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev