* [PATCH] Revert "drm/amdgpu: Verify bo size can fit framebuffer size on init."
@ 2021-04-29 20:50 Bas Nieuwenhuizen
2021-04-29 21:27 ` Mark Yacoub
2021-04-30 13:24 ` Michel Dänzer
0 siblings, 2 replies; 5+ messages in thread
From: Bas Nieuwenhuizen @ 2021-04-29 20:50 UTC (permalink / raw)
To: amd-gfx; +Cc: alexdeucher, markyacoub, Bas Nieuwenhuizen
This reverts commit f258907fdd835e1aed6d666b00cdd0f186676b7c.
Same problem as "drm/amdgpu: Verify bo size can fit framebuffer size",
but because it gets checked earlier it now only triggers on the
modifiers case.
There are a couple of reasons why the DRM core BO size check won't
work for AMDGPU, especially around DCC planes.
Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
---
For -fixes. Might have some conflicts with
"drm/amdgpu: Ensure that the modifier requested is supported by plane"
for amd-staging-drm-next
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 68 ++++-----------------
drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 4 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 8 ---
3 files changed, 15 insertions(+), 65 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 9a2f811450ed..cbe050436c7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -870,62 +870,17 @@ 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
@@ -936,13 +891,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;
- return ret;
+ goto fail;
}
}
ret = amdgpu_display_get_fb_info(rfb, &rfb->tiling_flags, &rfb->tmz_surface);
if (ret)
- return ret;
+ goto fail;
if (dev->mode_config.allow_fb_modifiers &&
!(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) {
@@ -950,7 +905,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);
- return ret;
+ goto fail;
}
}
@@ -961,6 +916,10 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev,
}
return 0;
+
+fail:
+ rfb->base.obj[0] = NULL;
+ return ret;
}
struct drm_framebuffer *
@@ -995,8 +954,7 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
return ERR_PTR(-ENOMEM);
}
- ret = amdgpu_display_gem_fb_verify_and_init(dev, amdgpu_fb, file_priv,
- mode_cmd, obj);
+ ret = amdgpu_display_framebuffer_init(dev, amdgpu_fb, 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 4c5c19820d37..24010cacf7d0 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_gem_fb_init(adev_to_drm(adev), &rfbdev->rfb,
- &mode_cmd, gobj);
+ ret = amdgpu_display_framebuffer_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 cb0b581bbce7..319cb19e1b99 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -602,14 +602,6 @@ 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.31.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] Revert "drm/amdgpu: Verify bo size can fit framebuffer size on init."
2021-04-29 20:50 [PATCH] Revert "drm/amdgpu: Verify bo size can fit framebuffer size on init." Bas Nieuwenhuizen
@ 2021-04-29 21:27 ` Mark Yacoub
2021-04-29 23:06 ` Bas Nieuwenhuizen
2021-04-30 13:24 ` Michel Dänzer
1 sibling, 1 reply; 5+ messages in thread
From: Mark Yacoub @ 2021-04-29 21:27 UTC (permalink / raw)
To: Bas Nieuwenhuizen; +Cc: alexdeucher, amd-gfx list
On Thu, Apr 29, 2021 at 4:50 PM Bas Nieuwenhuizen
<bas@basnieuwenhuizen.nl> wrote:
>
> This reverts commit f258907fdd835e1aed6d666b00cdd0f186676b7c.
>
> Same problem as "drm/amdgpu: Verify bo size can fit framebuffer size",
> but because it gets checked earlier it now only triggers on the
> modifiers case.
>
> There are a couple of reasons why the DRM core BO size check won't
> work for AMDGPU, especially around DCC planes.
>
Can you tell us more about those reasons? Last time this was reverted
due to a failure on ubuntu was due to a userspace bug that was fixed.
So I'm thinking we might wanna fix what broke instead of removing the
check.
> Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> ---
>
> For -fixes. Might have some conflicts with
> "drm/amdgpu: Ensure that the modifier requested is supported by plane"
> for amd-staging-drm-next
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 68 ++++-----------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 4 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 8 ---
> 3 files changed, 15 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 9a2f811450ed..cbe050436c7b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -870,62 +870,17 @@ 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
> @@ -936,13 +891,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;
> - return ret;
> + goto fail;
> }
> }
>
> ret = amdgpu_display_get_fb_info(rfb, &rfb->tiling_flags, &rfb->tmz_surface);
> if (ret)
> - return ret;
> + goto fail;
>
> if (dev->mode_config.allow_fb_modifiers &&
> !(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) {
> @@ -950,7 +905,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);
> - return ret;
> + goto fail;
> }
> }
>
> @@ -961,6 +916,10 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev,
> }
>
> return 0;
> +
> +fail:
> + rfb->base.obj[0] = NULL;
> + return ret;
> }
>
> struct drm_framebuffer *
> @@ -995,8 +954,7 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
> return ERR_PTR(-ENOMEM);
> }
>
> - ret = amdgpu_display_gem_fb_verify_and_init(dev, amdgpu_fb, file_priv,
> - mode_cmd, obj);
> + ret = amdgpu_display_framebuffer_init(dev, amdgpu_fb, 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 4c5c19820d37..24010cacf7d0 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_gem_fb_init(adev_to_drm(adev), &rfbdev->rfb,
> - &mode_cmd, gobj);
> + ret = amdgpu_display_framebuffer_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 cb0b581bbce7..319cb19e1b99 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -602,14 +602,6 @@ 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.31.1
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Revert "drm/amdgpu: Verify bo size can fit framebuffer size on init."
2021-04-29 21:27 ` Mark Yacoub
@ 2021-04-29 23:06 ` Bas Nieuwenhuizen
0 siblings, 0 replies; 5+ messages in thread
From: Bas Nieuwenhuizen @ 2021-04-29 23:06 UTC (permalink / raw)
To: Mark Yacoub; +Cc: Alex Deucher, amd-gfx list
On Thu, Apr 29, 2021 at 11:27 PM Mark Yacoub <markyacoub@chromium.org> wrote:
>
> On Thu, Apr 29, 2021 at 4:50 PM Bas Nieuwenhuizen
> <bas@basnieuwenhuizen.nl> wrote:
> >
> > This reverts commit f258907fdd835e1aed6d666b00cdd0f186676b7c.
> >
> > Same problem as "drm/amdgpu: Verify bo size can fit framebuffer size",
> > but because it gets checked earlier it now only triggers on the
> > modifiers case.
> >
> > There are a couple of reasons why the DRM core BO size check won't
> > work for AMDGPU, especially around DCC planes.
> >
> Can you tell us more about those reasons? Last time this was reverted
> due to a failure on ubuntu was due to a userspace bug that was fixed.
> So I'm thinking we might wanna fix what broke instead of removing the
> check.
Agree on having the check in the end, just wasn't sure if fixes (or
when I started looking at it, I thought stable) was the right place
given some of the tiling complexity.
So the core problems:
1) In the format structs we don't do set any of the tilesize / blocks
etc. to avoid having format arrays per modifier/GPU
2) The pitch on the main plane is pixel_pitch * bytes_per_pixel even
for tiled ...
3) The pitch for the DCC planes is really the pixel pitch of the main
surface that would be covered by it ...
1 is changeable by refactoring but sadly 2 and 3 are hard to change by
now (would need to bump the modifier version). And all 3 mean that the
default computation in the core drm helper is not the right check for
BO size.
> > Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> > ---
> >
> > For -fixes. Might have some conflicts with
> > "drm/amdgpu: Ensure that the modifier requested is supported by plane"
> > for amd-staging-drm-next
> >
> > drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 68 ++++-----------------
> > drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 4 +-
> > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 8 ---
> > 3 files changed, 15 insertions(+), 65 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > index 9a2f811450ed..cbe050436c7b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> > @@ -870,62 +870,17 @@ 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
> > @@ -936,13 +891,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;
> > - return ret;
> > + goto fail;
> > }
> > }
> >
> > ret = amdgpu_display_get_fb_info(rfb, &rfb->tiling_flags, &rfb->tmz_surface);
> > if (ret)
> > - return ret;
> > + goto fail;
> >
> > if (dev->mode_config.allow_fb_modifiers &&
> > !(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) {
> > @@ -950,7 +905,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);
> > - return ret;
> > + goto fail;
> > }
> > }
> >
> > @@ -961,6 +916,10 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev,
> > }
> >
> > return 0;
> > +
> > +fail:
> > + rfb->base.obj[0] = NULL;
> > + return ret;
> > }
> >
> > struct drm_framebuffer *
> > @@ -995,8 +954,7 @@ amdgpu_display_user_framebuffer_create(struct drm_device *dev,
> > return ERR_PTR(-ENOMEM);
> > }
> >
> > - ret = amdgpu_display_gem_fb_verify_and_init(dev, amdgpu_fb, file_priv,
> > - mode_cmd, obj);
> > + ret = amdgpu_display_framebuffer_init(dev, amdgpu_fb, 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 4c5c19820d37..24010cacf7d0 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_gem_fb_init(adev_to_drm(adev), &rfbdev->rfb,
> > - &mode_cmd, gobj);
> > + ret = amdgpu_display_framebuffer_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 cb0b581bbce7..319cb19e1b99 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> > @@ -602,14 +602,6 @@ 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.31.1
> >
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Revert "drm/amdgpu: Verify bo size can fit framebuffer size on init."
2021-04-29 20:50 [PATCH] Revert "drm/amdgpu: Verify bo size can fit framebuffer size on init." Bas Nieuwenhuizen
2021-04-29 21:27 ` Mark Yacoub
@ 2021-04-30 13:24 ` Michel Dänzer
2021-05-01 19:45 ` Pierre-Loup A. Griffais
1 sibling, 1 reply; 5+ messages in thread
From: Michel Dänzer @ 2021-04-30 13:24 UTC (permalink / raw)
To: Bas Nieuwenhuizen, amd-gfx; +Cc: alexdeucher, markyacoub, xinhui pan
On 2021-04-29 10:50 p.m., Bas Nieuwenhuizen wrote:
> This reverts commit f258907fdd835e1aed6d666b00cdd0f186676b7c.
>
> Same problem as "drm/amdgpu: Verify bo size can fit framebuffer size",
> but because it gets checked earlier it now only triggers on the
> modifiers case.
>
> There are a couple of reasons why the DRM core BO size check won't
> work for AMDGPU, especially around DCC planes.
>
> Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> ---
>
> For -fixes. Might have some conflicts with
> "drm/amdgpu: Ensure that the modifier requested is supported by plane"
> for amd-staging-drm-next
This interacts badly with 79fcd446e7e1 "drm/amdgpu: Fix memory leak", resulting in BOs getting destroyed prematurely and ensuing badness. To avoid that, at least the drm_gem_object_put call needs to be removed from the end of amdgpu_display_user_framebuffer_create needs to be removed as well, if not 79fcd446e7e1 reverted altogether.
I suspect 79fcd446e7e1 was at least partly a fix-up for f258907fdd83, to compensate for drm_gem_fb_init_with_funcs incrementing the BO reference counts.
--
Earthling Michel Dänzer | https://redhat.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Revert "drm/amdgpu: Verify bo size can fit framebuffer size on init."
2021-04-30 13:24 ` Michel Dänzer
@ 2021-05-01 19:45 ` Pierre-Loup A. Griffais
0 siblings, 0 replies; 5+ messages in thread
From: Pierre-Loup A. Griffais @ 2021-05-01 19:45 UTC (permalink / raw)
To: Michel Dänzer, Bas Nieuwenhuizen, amd-gfx
Cc: alexdeucher, markyacoub, xinhui pan
On 4/30/21 6:24 AM, Michel Dänzer wrote:
> On 2021-04-29 10:50 p.m., Bas Nieuwenhuizen wrote:
>> This reverts commit f258907fdd835e1aed6d666b00cdd0f186676b7c.
>>
>> Same problem as "drm/amdgpu: Verify bo size can fit framebuffer size",
>> but because it gets checked earlier it now only triggers on the
>> modifiers case.
>>
>> There are a couple of reasons why the DRM core BO size check won't
>> work for AMDGPU, especially around DCC planes.
>>
>> Signed-off-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
>> ---
>>
>> For -fixes. Might have some conflicts with
>> "drm/amdgpu: Ensure that the modifier requested is supported by plane"
>> for amd-staging-drm-next
>
> This interacts badly with 79fcd446e7e1 "drm/amdgpu: Fix memory leak", resulting in BOs getting destroyed prematurely and ensuing badness. To avoid that, at least the drm_gem_object_put call needs to be removed from the end of amdgpu_display_user_framebuffer_create needs to be removed as well, if not 79fcd446e7e1 reverted altogether.
>
>
> I suspect 79fcd446e7e1 was at least partly a fix-up for f258907fdd83, to compensate for drm_gem_fb_init_with_funcs incrementing the BO reference counts.
Thanks much for the pointer, reverting "drm/amdgpu: Fix memory leak" in
addition to the above patch yields a stable kernel on my side again.
Otherwise, things crash when gamescope attempts to flip its cursor
surface onto the screen.
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-01 19:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-29 20:50 [PATCH] Revert "drm/amdgpu: Verify bo size can fit framebuffer size on init." Bas Nieuwenhuizen
2021-04-29 21:27 ` Mark Yacoub
2021-04-29 23:06 ` Bas Nieuwenhuizen
2021-04-30 13:24 ` Michel Dänzer
2021-05-01 19:45 ` Pierre-Loup A. Griffais
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).