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

Reply via email to