On Thu, Mar 4, 2021 at 2:15 PM Mark Yacoub <markyacoub@xxxxxxxxxxxx> wrote:
>
> From: Mark Yacoub <markyacoub@xxxxxxxxxx>
>
> To initialize the framebuffer, use 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.
>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Cc: "Christian König" <christian.koenig@xxxxxxx>
> Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
> Signed-off-by: Mark Yacoub <markyacoub@xxxxxxxxxxxx>
Thanks for the patch. Just a few minor comments below.
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 66 ++++++++++++++++-----
> drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 4 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 8 +++
> 3 files changed, 62 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 48cb33e5b3826..554038e5bbf6a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -870,18 +870,59 @@ static int amdgpu_display_get_fb_info(const struct amdgpu_framebuffer *amdgpu_fb
> return r;
> }
>
> -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 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, i;
> + int ret;
Please add a new line here.
> 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;
> + 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.
Please change this to use C style comments.
> + 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;
New line here.
> /*
> * This needs to happen before modifier conversion as that might change
> * the number of planes.
> @@ -891,13 +932,13 @@ 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)) {
> @@ -905,7 +946,7 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev,
> if (ret) {
> drm_dbg_kms(dev, "Failed to convert tiling flags 0x%llX to a modifier",
> rfb->tiling_flags);
> - goto fail;
> + return ret;
> }
> }
>
> @@ -915,10 +956,6 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev,
> }
>
> return 0;
> -
> -fail:
> - rfb->base.obj[0] = NULL;
> - return ret;
> }
>
> struct drm_framebuffer *
> @@ -953,7 +990,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);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> index 0bf7d36c6686d..22157bd7017c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> @@ -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;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 319cb19e1b99f..cb0b581bbce7b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -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,
> --
> 2.30.1.766.gb4fecdf3b7-goog
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
