On Thu, Jul 30, 2026 at 3:20 AM David Rosca <[email protected]> wrote:
>
> This isn't doing anything useful. For H264/H265 DPB size it also incorrectly
> forces lower bound on number of reference frames, but UVD FW only needs
> as many slots as the decoded stream actually uses.
>
> Signed-off-by: David Rosca <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 239 +-----------------------
> 1 file changed, 4 insertions(+), 235 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> index e8b0c62f72be..f410a16a4f53 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> @@ -103,9 +103,6 @@ struct amdgpu_uvd_cs_ctx {
>
> /* does the IB has a msg command */
> bool has_msg_cmd;
> -
> - /* minimum buffer sizes */
> - unsigned int *buf_sizes;
> };
>
> #ifdef CONFIG_DRM_AMDGPU_SI
> @@ -629,209 +626,6 @@ static int amdgpu_uvd_cs_pass1(struct amdgpu_uvd_cs_ctx
> *ctx)
> return r;
> }
>
> -/**
> - * amdgpu_uvd_cs_msg_decode - handle UVD decode message
> - *
> - * @adev: amdgpu_device pointer
> - * @msg: pointer to message structure
> - * @buf_sizes: placeholder to put the different buffer lengths
> - *
> - * Peek into the decode message and calculate the necessary buffer sizes.
> - */
> -static int amdgpu_uvd_cs_msg_decode(struct amdgpu_device *adev, uint32_t
> *msg,
> - unsigned int buf_sizes[])
> -{
> - unsigned int stream_type = msg[4];
> - unsigned int width = msg[6];
> - unsigned int height = msg[7];
> - unsigned int dpb_size = msg[9];
> - unsigned int pitch = msg[28];
> - unsigned int level = msg[57];
> -
> - unsigned int width_in_mb = width / 16;
> - unsigned int height_in_mb = ALIGN(height / 16, 2);
> - unsigned int fs_in_mb = width_in_mb * height_in_mb;
> -
> - unsigned int image_size, tmp, min_dpb_size, num_dpb_buffer;
> - unsigned int min_ctx_size = ~0;
> -
> - /* Reject invalid dimensions to prevent division by zero */
> - if (width < 16 || height < 16) {
> - dev_WARN_ONCE(adev->dev, 1,
> - "Invalid UVD decoding dimensions (%dx%d)!\n",
> - width, height);
> - return -EINVAL;
> - }
> -
> - image_size = width * height;
> - image_size += image_size / 2;
> - image_size = ALIGN(image_size, 1024);
> -
> - switch (stream_type) {
> - case 0: /* H264 */
> - switch (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++;
> - if (num_dpb_buffer > 17)
> - num_dpb_buffer = 17;
> -
> - /* reference picture buffer */
> - min_dpb_size = image_size * num_dpb_buffer;
> -
> - /* macroblock context buffer */
> - min_dpb_size += width_in_mb * height_in_mb * num_dpb_buffer *
> 192;
> -
> - /* IT surface buffer */
> - min_dpb_size += width_in_mb * height_in_mb * 32;
> - break;
> -
> - case 1: /* VC1 */
> -
> - /* reference picture buffer */
> - min_dpb_size = image_size * 3;
> -
> - /* CONTEXT_BUFFER */
> - min_dpb_size += width_in_mb * height_in_mb * 128;
> -
> - /* IT surface buffer */
> - min_dpb_size += width_in_mb * 64;
> -
> - /* DB surface buffer */
> - min_dpb_size += width_in_mb * 128;
> -
> - /* BP */
> - tmp = max(width_in_mb, height_in_mb);
> - min_dpb_size += ALIGN(tmp * 7 * 16, 64);
> - break;
> -
> - case 3: /* MPEG2 */
> -
> - /* reference picture buffer */
> - min_dpb_size = image_size * 3;
> - break;
> -
> - case 4: /* MPEG4 */
> -
> - /* reference picture buffer */
> - min_dpb_size = image_size * 3;
> -
> - /* CM */
> - min_dpb_size += width_in_mb * height_in_mb * 64;
> -
> - /* IT surface buffer */
> - min_dpb_size += ALIGN(width_in_mb * height_in_mb * 32, 64);
> - break;
> -
> - case 7: /* H264 Perf */
> - switch (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++;
> - if (num_dpb_buffer > 17)
> - num_dpb_buffer = 17;
> -
> - /* reference picture buffer */
> - min_dpb_size = image_size * num_dpb_buffer;
> -
> - if (!adev->uvd.use_ctx_buf) {
> - /* macroblock context buffer */
> - min_dpb_size +=
> - width_in_mb * height_in_mb * num_dpb_buffer *
> 192;
> -
> - /* IT surface buffer */
> - min_dpb_size += width_in_mb * height_in_mb * 32;
> - } else {
> - /* macroblock context buffer */
> - min_ctx_size =
> - width_in_mb * height_in_mb * num_dpb_buffer *
> 192;
> - }
> - break;
> -
> - case 8: /* MJPEG */
> - min_dpb_size = 0;
> - break;
> -
> - case 16: /* H265 */
> - image_size = (ALIGN(width, 16) * ALIGN(height, 16) * 3) / 2;
> - image_size = ALIGN(image_size, 256);
> -
> - num_dpb_buffer = (le32_to_cpu(msg[59]) & 0xff) + 2;
> - min_dpb_size = image_size * num_dpb_buffer;
> - min_ctx_size = ((width + 255) / 16) * ((height + 255) / 16)
> - * 16 * num_dpb_buffer + 52 * 1024;
> - break;
> -
> - default:
> - DRM_ERROR("UVD codec not handled %d!\n", stream_type);
> - return -EINVAL;
> - }
> -
> - if (width > pitch) {
> - DRM_ERROR("Invalid UVD decoding target pitch!\n");
> - return -EINVAL;
> - }
> -
> - if (dpb_size < min_dpb_size) {
> - DRM_ERROR("Invalid dpb_size in UVD message (%d / %d)!\n",
> - dpb_size, min_dpb_size);
> - return -EINVAL;
> - }
> -
> - buf_sizes[0x1] = dpb_size;
> - buf_sizes[0x2] = image_size;
> - buf_sizes[0x4] = min_ctx_size;
> - /* store image width to adjust nb memory pstate */
> - adev->uvd.decode_image_width = width;
> - return 0;
Aren't these checks necessary on systems without GPUVM support to make
sure UVD is bounded to its set of buffers? Otherwise, I think we'll
just be introducing a potential security hole.
Alex
> -}
> -
> /**
> * amdgpu_uvd_cs_msg - handle UVD message
> *
> @@ -875,7 +669,7 @@ static int amdgpu_uvd_cs_msg(struct amdgpu_uvd_cs_ctx
> *ctx,
>
> switch (msg_type) {
> case 0:
> - /* it's a create msg, calc image size (width * height) */
> + /* it's a create msg */
> amdgpu_bo_kunmap(bo);
>
> /* try to alloc a new handle */
> @@ -896,11 +690,9 @@ static int amdgpu_uvd_cs_msg(struct amdgpu_uvd_cs_ctx
> *ctx,
> return -ENOSPC;
>
> case 1:
> - /* it's a decode msg, calc buffer sizes */
> - r = amdgpu_uvd_cs_msg_decode(adev, msg, ctx->buf_sizes);
> + /* it's a decode msg */
> + adev->uvd.decode_image_width = msg[6];
> amdgpu_bo_kunmap(bo);
> - if (r)
> - return r;
>
> /* validate the handle */
> for (i = 0; i < adev->uvd.max_handles; ++i) {
> @@ -965,22 +757,7 @@ static int amdgpu_uvd_cs_pass2(struct amdgpu_uvd_cs_ctx
> *ctx)
> amdgpu_ib_set_value(ctx->ib, ctx->data1, upper_32_bits(start));
>
> cmd = amdgpu_ib_get_value(ctx->ib, ctx->idx) >> 1;
> - if (cmd < 0x4) {
> - if ((end - start) < ctx->buf_sizes[cmd]) {
> - DRM_ERROR("buffer (%d) to small (%d / %d)!\n", cmd,
> - (unsigned int)(end - start),
> - ctx->buf_sizes[cmd]);
> - return -EINVAL;
> - }
> -
> - } else if (cmd == 0x206) {
> - if ((end - start) < ctx->buf_sizes[4]) {
> - DRM_ERROR("buffer (%d) to small (%d / %d)!\n", cmd,
> - (unsigned int)(end - start),
> - ctx->buf_sizes[4]);
> - return -EINVAL;
> - }
> - } else if ((cmd != 0x100) && (cmd != 0x204)) {
> + if ((cmd >= 0x4) && (cmd != 0x100) && (cmd != 0x204) && (cmd !=
> 0x206)) {
> DRM_ERROR("invalid UVD command %X!\n", cmd);
> return -EINVAL;
> }
> @@ -1109,13 +886,6 @@ int amdgpu_uvd_ring_parse_cs(struct amdgpu_cs_parser
> *parser,
> struct amdgpu_ib *ib)
> {
> struct amdgpu_uvd_cs_ctx ctx = {};
> - unsigned int buf_sizes[] = {
> - [0x00000000] = 2048,
> - [0x00000001] = 0xFFFFFFFF,
> - [0x00000002] = 0xFFFFFFFF,
> - [0x00000003] = 2048,
> - [0x00000004] = 0xFFFFFFFF,
> - };
> int r;
>
> job->vm = NULL;
> @@ -1127,7 +897,6 @@ int amdgpu_uvd_ring_parse_cs(struct amdgpu_cs_parser
> *parser,
> }
>
> ctx.parser = parser;
> - ctx.buf_sizes = buf_sizes;
> ctx.ib = ib;
>
> /* first round only required on chips without UVD 64 bit address
> support */
> --
> 2.43.0
>
