Skip to content

Commit

Permalink
drm/amdgpu: Verify bo size can fit framebuffer size on init.
Browse files Browse the repository at this point in the history
To initialize the framebuffer, call drm_gem_fb_init_with_funcs which
verifies that the BO size can fit the FB size by calculating the minimum
expected size of each plane.

The bug was caught using igt-gpu-tools test: kms_addfb_basic.too-high
and kms_addfb_basic.bo-too-small

Tested on ChromeOS Zork by turning on the display and running a YT
video.

=== Changes from v1 ===
1. Added new line under declarations.
2. Use C style comment.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
  • Loading branch information
Mark Yacoub authored and Alex Deucher committed Mar 24, 2021
1 parent c153401 commit f258907
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 15 deletions.
68 changes: 55 additions & 13 deletions drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
Original file line number Diff line number Diff line change
Expand Up @@ -870,17 +870,62 @@ static int amdgpu_display_get_fb_info(const struct amdgpu_framebuffer *amdgpu_fb
return r;
}

int amdgpu_display_gem_fb_init(struct drm_device *dev,
struct amdgpu_framebuffer *rfb,
const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *obj)
{
int ret;

rfb->base.obj[0] = obj;
drm_helper_mode_fill_fb_struct(dev, &rfb->base, mode_cmd);
ret = drm_framebuffer_init(dev, &rfb->base, &amdgpu_fb_funcs);
if (ret)
goto err;

ret = amdgpu_display_framebuffer_init(dev, rfb, mode_cmd, obj);
if (ret)
goto err;

return 0;
err:
drm_err(dev, "Failed to init gem fb: %d\n", ret);
rfb->base.obj[0] = NULL;
return ret;
}

int amdgpu_display_gem_fb_verify_and_init(
struct drm_device *dev, struct amdgpu_framebuffer *rfb,
struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *obj)
{
int ret;

rfb->base.obj[0] = obj;

/* Verify that bo size can fit the fb size. */
ret = drm_gem_fb_init_with_funcs(dev, &rfb->base, file_priv, mode_cmd,
&amdgpu_fb_funcs);
if (ret)
goto err;

ret = amdgpu_display_framebuffer_init(dev, rfb, mode_cmd, obj);
if (ret)
goto err;

return 0;
err:
drm_err(dev, "Failed to verify and init gem fb: %d\n", ret);
rfb->base.obj[0] = NULL;
return ret;
}

int amdgpu_display_framebuffer_init(struct drm_device *dev,
struct amdgpu_framebuffer *rfb,
const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *obj)
{
int ret, i;
rfb->base.obj[0] = obj;
drm_helper_mode_fill_fb_struct(dev, &rfb->base, mode_cmd);
ret = drm_framebuffer_init(dev, &rfb->base, &amdgpu_fb_funcs);
if (ret)
goto fail;

/*
* This needs to happen before modifier conversion as that might change
Expand All @@ -891,21 +936,21 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev,
drm_dbg_kms(dev, "Plane 0 and %d have different BOs: %u vs. %u\n",
i, mode_cmd->handles[0], mode_cmd->handles[i]);
ret = -EINVAL;
goto fail;
return ret;
}
}

ret = amdgpu_display_get_fb_info(rfb, &rfb->tiling_flags, &rfb->tmz_surface);
if (ret)
goto fail;
return ret;

if (dev->mode_config.allow_fb_modifiers &&
!(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) {
ret = convert_tiling_flags_to_modifier(rfb);
if (ret) {
drm_dbg_kms(dev, "Failed to convert tiling flags 0x%llX to a modifier",
rfb->tiling_flags);
goto fail;
return ret;
}
}

Expand All @@ -915,10 +960,6 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev,
}

return 0;

fail:
rfb->base.obj[0] = NULL;
return ret;
}

struct drm_framebuffer *
Expand Down Expand Up @@ -953,7 +994,8 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
return ERR_PTR(-ENOMEM);
}

ret = amdgpu_display_framebuffer_init(dev, amdgpu_fb, mode_cmd, obj);
ret = amdgpu_display_gem_fb_verify_and_init(dev, amdgpu_fb, file_priv,
mode_cmd, obj);
if (ret) {
kfree(amdgpu_fb);
drm_gem_object_put(obj);
Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,8 @@ static int amdgpufb_create(struct drm_fb_helper *helper,
goto out;
}

ret = amdgpu_display_framebuffer_init(adev_to_drm(adev), &rfbdev->rfb,
&mode_cmd, gobj);
ret = amdgpu_display_gem_fb_init(adev_to_drm(adev), &rfbdev->rfb,
&mode_cmd, gobj);
if (ret) {
DRM_ERROR("failed to initialize framebuffer %d\n", ret);
goto out;
Expand Down
8 changes: 8 additions & 0 deletions drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,14 @@ int amdgpu_display_get_crtc_scanoutpos(struct drm_device *dev,
int *hpos, ktime_t *stime, ktime_t *etime,
const struct drm_display_mode *mode);

int amdgpu_display_gem_fb_init(struct drm_device *dev,
struct amdgpu_framebuffer *rfb,
const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *obj);
int amdgpu_display_gem_fb_verify_and_init(
struct drm_device *dev, struct amdgpu_framebuffer *rfb,
struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
struct drm_gem_object *obj);
int amdgpu_display_framebuffer_init(struct drm_device *dev,
struct amdgpu_framebuffer *rfb,
const struct drm_mode_fb_cmd2 *mode_cmd,
Expand Down

0 comments on commit f258907

Please sign in to comment.