* [PATCH] drm/i915/fbdev: Call intel_unpin_fb_obj() on release
@ 2016-04-22 7:47 Chris Wilson
2016-04-22 8:22 ` ✗ Fi.CI.BAT: warning for " Patchwork
2016-04-22 8:47 ` [PATCH] " Tvrtko Ursulin
0 siblings, 2 replies; 6+ messages in thread
From: Chris Wilson @ 2016-04-22 7:47 UTC (permalink / raw)
To: intel-gfx; +Cc: Mika Kuoppala
When releasing the intel_fbdev, we should unpin the framebuffer that we
pinned during construction.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
I'm hoping this explains some of the flip-flip in DMESG-WARN for
drv_module_reload_basic...
-Chris
---
drivers/gpu/drm/i915/intel_display.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_fbdev.c | 7 ++++++-
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ff60241b1f76..a59fe7b032b0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2309,7 +2309,7 @@ err_pm:
return ret;
}
-static void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
+void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
{
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct i915_ggtt_view view;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index beed9e81252b..e336b19adb37 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1161,6 +1161,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
struct drm_modeset_acquire_ctx *ctx);
int intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
unsigned int rotation);
+void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
struct drm_framebuffer *
__intel_framebuffer_create(struct drm_device *dev,
struct drm_mode_fb_cmd2 *mode_cmd,
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 79ac202f3870..c3c265d6f521 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -287,7 +287,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
out_destroy_fbi:
drm_fb_helper_release_fbi(helper);
out_unpin:
- i915_gem_object_ggtt_unpin(obj);
+ intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0));
out_unlock:
mutex_unlock(&dev->struct_mutex);
return ret;
@@ -551,6 +551,11 @@ static void intel_fbdev_destroy(struct drm_device *dev,
if (ifbdev->fb) {
drm_framebuffer_unregister_private(&ifbdev->fb->base);
+
+ mutex_lock(&dev->struct_mutex);
+ intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0));
+ mutex_unlock(&dev->struct_mutex);
+
drm_framebuffer_remove(&ifbdev->fb->base);
}
}
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915/fbdev: Call intel_unpin_fb_obj() on release
2016-04-22 7:47 [PATCH] drm/i915/fbdev: Call intel_unpin_fb_obj() on release Chris Wilson
@ 2016-04-22 8:22 ` Patchwork
2016-04-22 8:47 ` [PATCH] " Tvrtko Ursulin
1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-04-22 8:22 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/fbdev: Call intel_unpin_fb_obj() on release
URL : https://patchwork.freedesktop.org/series/6137/
State : warning
== Summary ==
Series 6137v1 drm/i915/fbdev: Call intel_unpin_fb_obj() on release
http://patchwork.freedesktop.org/api/1.0/series/6137/revisions/1/mbox/
Test gem_busy:
Subgroup basic-blt:
skip -> PASS (bsw-nuc-2)
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
fail -> PASS (ilk-hp8440p) UNSTABLE
Test kms_force_connector_basic:
Subgroup force-load-detect:
pass -> SKIP (ilk-hp8440p)
bdw-nuci7 total:193 pass:181 dwarn:0 dfail:0 fail:0 skip:12
bdw-ultra total:193 pass:170 dwarn:0 dfail:0 fail:0 skip:23
bsw-nuc-2 total:192 pass:153 dwarn:0 dfail:0 fail:0 skip:39
byt-nuc total:192 pass:154 dwarn:0 dfail:0 fail:0 skip:38
hsw-brixbox total:193 pass:169 dwarn:0 dfail:0 fail:0 skip:24
ilk-hp8440p total:193 pass:135 dwarn:0 dfail:0 fail:0 skip:58
ivb-t430s total:193 pass:165 dwarn:0 dfail:0 fail:0 skip:28
skl-i7k-2 total:193 pass:168 dwarn:0 dfail:0 fail:0 skip:25
skl-nuci5 total:193 pass:182 dwarn:0 dfail:0 fail:0 skip:11
snb-dellxps total:193 pass:155 dwarn:0 dfail:0 fail:0 skip:38
snb-x220t total:193 pass:155 dwarn:0 dfail:0 fail:1 skip:37
Results at /archive/results/CI_IGT_test/Patchwork_1985/
d5b5101bd09a7b7e48b1cd78fe8f8a40b21d4deb drm-intel-nightly: 2016y-04m-21d-16h-37m-06s UTC integration manifest
c8194b4 drm/i915/fbdev: Call intel_unpin_fb_obj() on release
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/fbdev: Call intel_unpin_fb_obj() on release
2016-04-22 7:47 [PATCH] drm/i915/fbdev: Call intel_unpin_fb_obj() on release Chris Wilson
2016-04-22 8:22 ` ✗ Fi.CI.BAT: warning for " Patchwork
@ 2016-04-22 8:47 ` Tvrtko Ursulin
2016-04-22 9:00 ` Chris Wilson
1 sibling, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-04-22 8:47 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: Mika Kuoppala
On 22/04/16 08:47, Chris Wilson wrote:
> When releasing the intel_fbdev, we should unpin the framebuffer that we
> pinned during construction.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> I'm hoping this explains some of the flip-flip in DMESG-WARN for
> drv_module_reload_basic...
> -Chris
> ---
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_fbdev.c | 7 ++++++-
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ff60241b1f76..a59fe7b032b0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2309,7 +2309,7 @@ err_pm:
> return ret;
> }
>
> -static void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
> +void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
> {
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> struct i915_ggtt_view view;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index beed9e81252b..e336b19adb37 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1161,6 +1161,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
> struct drm_modeset_acquire_ctx *ctx);
> int intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
> unsigned int rotation);
> +void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
> struct drm_framebuffer *
> __intel_framebuffer_create(struct drm_device *dev,
> struct drm_mode_fb_cmd2 *mode_cmd,
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 79ac202f3870..c3c265d6f521 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -287,7 +287,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
> out_destroy_fbi:
> drm_fb_helper_release_fbi(helper);
> out_unpin:
> - i915_gem_object_ggtt_unpin(obj);
> + intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0));
> out_unlock:
> mutex_unlock(&dev->struct_mutex);
> return ret;
> @@ -551,6 +551,11 @@ static void intel_fbdev_destroy(struct drm_device *dev,
>
> if (ifbdev->fb) {
> drm_framebuffer_unregister_private(&ifbdev->fb->base);
> +
> + mutex_lock(&dev->struct_mutex);
> + intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0));
> + mutex_unlock(&dev->struct_mutex);
> +
> drm_framebuffer_remove(&ifbdev->fb->base);
> }
> }
>
What is new to need this? I looked at the related code, and maybe got
the wrong conclusions, but it looks like that warning in drm_mm_takedown
would fire all the time if this drm_framebuffer_remove was not the last
reference and wouldn't do the unpinning?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/fbdev: Call intel_unpin_fb_obj() on release
2016-04-22 8:47 ` [PATCH] " Tvrtko Ursulin
@ 2016-04-22 9:00 ` Chris Wilson
2016-04-22 11:59 ` Tvrtko Ursulin
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2016-04-22 9:00 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx, Mika Kuoppala
On Fri, Apr 22, 2016 at 09:47:47AM +0100, Tvrtko Ursulin wrote:
>
> On 22/04/16 08:47, Chris Wilson wrote:
> >When releasing the intel_fbdev, we should unpin the framebuffer that we
> >pinned during construction.
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >---
> >I'm hoping this explains some of the flip-flip in DMESG-WARN for
> >drv_module_reload_basic...
> >-Chris
> >---
> > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_fbdev.c | 7 ++++++-
> > 3 files changed, 8 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >index ff60241b1f76..a59fe7b032b0 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -2309,7 +2309,7 @@ err_pm:
> > return ret;
> > }
> >
> >-static void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
> >+void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
> > {
> > struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> > struct i915_ggtt_view view;
> >diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >index beed9e81252b..e336b19adb37 100644
> >--- a/drivers/gpu/drm/i915/intel_drv.h
> >+++ b/drivers/gpu/drm/i915/intel_drv.h
> >@@ -1161,6 +1161,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
> > struct drm_modeset_acquire_ctx *ctx);
> > int intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
> > unsigned int rotation);
> >+void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
> > struct drm_framebuffer *
> > __intel_framebuffer_create(struct drm_device *dev,
> > struct drm_mode_fb_cmd2 *mode_cmd,
> >diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> >index 79ac202f3870..c3c265d6f521 100644
> >--- a/drivers/gpu/drm/i915/intel_fbdev.c
> >+++ b/drivers/gpu/drm/i915/intel_fbdev.c
> >@@ -287,7 +287,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
> > out_destroy_fbi:
> > drm_fb_helper_release_fbi(helper);
> > out_unpin:
> >- i915_gem_object_ggtt_unpin(obj);
> >+ intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0));
> > out_unlock:
> > mutex_unlock(&dev->struct_mutex);
> > return ret;
> >@@ -551,6 +551,11 @@ static void intel_fbdev_destroy(struct drm_device *dev,
> >
> > if (ifbdev->fb) {
> > drm_framebuffer_unregister_private(&ifbdev->fb->base);
> >+
> >+ mutex_lock(&dev->struct_mutex);
> >+ intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0));
> >+ mutex_unlock(&dev->struct_mutex);
> >+
> > drm_framebuffer_remove(&ifbdev->fb->base);
> > }
> > }
> >
>
> What is new to need this? I looked at the related code, and maybe
> got the wrong conclusions, but it looks like that warning in
> drm_mm_takedown would fire all the time if this
> drm_framebuffer_remove was not the last reference and wouldn't do
> the unpinning?
I've got an extra warning in my tree for the untracked vma (which fires
ofc). However, it doesn't fix the issue of !fbdev leaking memory. So,
yes, it is just a warning fix from later that I'm optimistically
applying now.
Something about CI results being hidden and not seeing the same failures
locally...
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/fbdev: Call intel_unpin_fb_obj() on release
2016-04-22 9:00 ` Chris Wilson
@ 2016-04-22 11:59 ` Tvrtko Ursulin
2016-04-22 12:11 ` Chris Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-04-22 11:59 UTC (permalink / raw)
To: Chris Wilson, intel-gfx, Joonas Lahtinen, Mika Kuoppala
On 22/04/16 10:00, Chris Wilson wrote:
> On Fri, Apr 22, 2016 at 09:47:47AM +0100, Tvrtko Ursulin wrote:
>>
>> On 22/04/16 08:47, Chris Wilson wrote:
>>> When releasing the intel_fbdev, we should unpin the framebuffer that we
>>> pinned during construction.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>> I'm hoping this explains some of the flip-flip in DMESG-WARN for
>>> drv_module_reload_basic...
>>> -Chris
>>> ---
>>> drivers/gpu/drm/i915/intel_display.c | 2 +-
>>> drivers/gpu/drm/i915/intel_drv.h | 1 +
>>> drivers/gpu/drm/i915/intel_fbdev.c | 7 ++++++-
>>> 3 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index ff60241b1f76..a59fe7b032b0 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -2309,7 +2309,7 @@ err_pm:
>>> return ret;
>>> }
>>>
>>> -static void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
>>> +void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
>>> {
>>> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>>> struct i915_ggtt_view view;
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index beed9e81252b..e336b19adb37 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -1161,6 +1161,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
>>> struct drm_modeset_acquire_ctx *ctx);
>>> int intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
>>> unsigned int rotation);
>>> +void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
>>> struct drm_framebuffer *
>>> __intel_framebuffer_create(struct drm_device *dev,
>>> struct drm_mode_fb_cmd2 *mode_cmd,
>>> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
>>> index 79ac202f3870..c3c265d6f521 100644
>>> --- a/drivers/gpu/drm/i915/intel_fbdev.c
>>> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
>>> @@ -287,7 +287,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
>>> out_destroy_fbi:
>>> drm_fb_helper_release_fbi(helper);
>>> out_unpin:
>>> - i915_gem_object_ggtt_unpin(obj);
>>> + intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0));
>>> out_unlock:
>>> mutex_unlock(&dev->struct_mutex);
>>> return ret;
>>> @@ -551,6 +551,11 @@ static void intel_fbdev_destroy(struct drm_device *dev,
>>>
>>> if (ifbdev->fb) {
>>> drm_framebuffer_unregister_private(&ifbdev->fb->base);
>>> +
>>> + mutex_lock(&dev->struct_mutex);
>>> + intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0));
>>> + mutex_unlock(&dev->struct_mutex);
>>> +
>>> drm_framebuffer_remove(&ifbdev->fb->base);
>>> }
>>> }
>>>
>>
>> What is new to need this? I looked at the related code, and maybe
>> got the wrong conclusions, but it looks like that warning in
>> drm_mm_takedown would fire all the time if this
>> drm_framebuffer_remove was not the last reference and wouldn't do
>> the unpinning?
>
> I've got an extra warning in my tree for the untracked vma (which fires
> ofc). However, it doesn't fix the issue of !fbdev leaking memory. So,
> yes, it is just a warning fix from later that I'm optimistically
> applying now.
>
> Something about CI results being hidden and not seeing the same failures
> locally...
Didn't really understand any of that. Is think blocking the premature
unpin series? Is the warning only appearing with that series or is it
something already present?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/fbdev: Call intel_unpin_fb_obj() on release
2016-04-22 11:59 ` Tvrtko Ursulin
@ 2016-04-22 12:11 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2016-04-22 12:11 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx, Mika Kuoppala
On Fri, Apr 22, 2016 at 12:59:12PM +0100, Tvrtko Ursulin wrote:
>
> On 22/04/16 10:00, Chris Wilson wrote:
> >Something about CI results being hidden and not seeing the same failures
> >locally...
>
> Didn't really understand any of that. Is think blocking the
> premature unpin series? Is the warning only appearing with that
> series or is it something already present?
I meant I can't see the results. The WARN is coming on a different set
of machines, but only seems to be after this series (from watching CI
mails) and previously the WARN seemed to be pertinent (i.e. it was a
drm_mm memory manager warning.
We are missing something like:
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index ff7d563c5623..01cd061ab573 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -552,6 +552,8 @@ void i915_gem_context_fini(struct drm_device *dev)
i915_gem_context_unreference(dctx);
dev_priv->kernel_context = NULL;
+ WARN_ON(!list_empty(&dev_priv->context_list));
+
ida_destroy(&dev_priv->context_hw_ida);
}
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-22 12:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-22 7:47 [PATCH] drm/i915/fbdev: Call intel_unpin_fb_obj() on release Chris Wilson
2016-04-22 8:22 ` ✗ Fi.CI.BAT: warning for " Patchwork
2016-04-22 8:47 ` [PATCH] " Tvrtko Ursulin
2016-04-22 9:00 ` Chris Wilson
2016-04-22 11:59 ` Tvrtko Ursulin
2016-04-22 12:11 ` Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.