* [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
@ 2015-03-26 12:39 Tvrtko Ursulin
2015-03-26 13:30 ` Ville Syrjälä
2015-03-26 20:13 ` shuang.he
0 siblings, 2 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2015-03-26 12:39 UTC (permalink / raw)
To: Intel-gfx
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
intel_user_framebuffer_destroy() requires the struct_mutex for its
object bookkeeping, so this means that all calls to
drm_framebuffer_unreference must be held without that lock.
This is a simplified version of the identically named patch by Chris Wilson.
References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cb50854..0788507 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14020,11 +14020,21 @@ void intel_modeset_gem_init(struct drm_device *dev)
c->primary->fb,
c->primary->state,
NULL)) {
+ /*
+ * We must drop struct_mutex when calling
+ * drm_framebuffer_unreference and it is safe to do so
+ * because it is not needed at this point anyway.
+ * At this stage the driver is still single-threaded and
+ * we are taking it only to silence a warning in
+ * intel_pin_and_fence_fb_obj.
+ */
+ mutex_unlock(&dev->struct_mutex);
DRM_ERROR("failed to pin boot fb on pipe %d\n",
to_intel_crtc(c)->pipe);
drm_framebuffer_unreference(c->primary->fb);
c->primary->fb = NULL;
update_state_fb(c->primary);
+ mutex_lock(&dev->struct_mutex);
}
}
mutex_unlock(&dev->struct_mutex);
--
2.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
2015-03-26 12:39 [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover Tvrtko Ursulin
@ 2015-03-26 13:30 ` Ville Syrjälä
2015-03-26 14:05 ` Tvrtko Ursulin
2015-03-26 20:13 ` shuang.he
1 sibling, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2015-03-26 13:30 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Thu, Mar 26, 2015 at 12:39:40PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> intel_user_framebuffer_destroy() requires the struct_mutex for its
> object bookkeeping, so this means that all calls to
> drm_framebuffer_unreference must be held without that lock.
>
> This is a simplified version of the identically named patch by Chris Wilson.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cb50854..0788507 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14020,11 +14020,21 @@ void intel_modeset_gem_init(struct drm_device *dev)
> c->primary->fb,
> c->primary->state,
> NULL)) {
> + /*
> + * We must drop struct_mutex when calling
> + * drm_framebuffer_unreference and it is safe to do so
> + * because it is not needed at this point anyway.
> + * At this stage the driver is still single-threaded and
> + * we are taking it only to silence a warning in
> + * intel_pin_and_fence_fb_obj.
> + */
> + mutex_unlock(&dev->struct_mutex);
> DRM_ERROR("failed to pin boot fb on pipe %d\n",
> to_intel_crtc(c)->pipe);
> drm_framebuffer_unreference(c->primary->fb);
> c->primary->fb = NULL;
> update_state_fb(c->primary);
> + mutex_lock(&dev->struct_mutex);
> }
> }
> mutex_unlock(&dev->struct_mutex);
Just grab the mutex around the pin_and_fence inside the loop. It doesn't
protect anything else.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
2015-03-26 13:30 ` Ville Syrjälä
@ 2015-03-26 14:05 ` Tvrtko Ursulin
2015-04-13 12:09 ` Jani Nikula
0 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2015-03-26 14:05 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel-gfx
On 03/26/2015 01:30 PM, Ville Syrjälä wrote:
> On Thu, Mar 26, 2015 at 12:39:40PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> intel_user_framebuffer_destroy() requires the struct_mutex for its
>> object bookkeeping, so this means that all calls to
>> drm_framebuffer_unreference must be held without that lock.
>>
>> This is a simplified version of the identically named patch by Chris Wilson.
>>
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index cb50854..0788507 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -14020,11 +14020,21 @@ void intel_modeset_gem_init(struct drm_device *dev)
>> c->primary->fb,
>> c->primary->state,
>> NULL)) {
>> + /*
>> + * We must drop struct_mutex when calling
>> + * drm_framebuffer_unreference and it is safe to do so
>> + * because it is not needed at this point anyway.
>> + * At this stage the driver is still single-threaded and
>> + * we are taking it only to silence a warning in
>> + * intel_pin_and_fence_fb_obj.
>> + */
>> + mutex_unlock(&dev->struct_mutex);
>> DRM_ERROR("failed to pin boot fb on pipe %d\n",
>> to_intel_crtc(c)->pipe);
>> drm_framebuffer_unreference(c->primary->fb);
>> c->primary->fb = NULL;
>> update_state_fb(c->primary);
>> + mutex_lock(&dev->struct_mutex);
>> }
>> }
>> mutex_unlock(&dev->struct_mutex);
>
> Just grab the mutex around the pin_and_fence inside the loop. It doesn't
> protect anything else.
Well the comment says so, but this way it only grabs and releases it
once if there are multiple active crtcs and nothing fails. So I was
hoping the comment was enough to explain the reality, even though the
other option would be more obvious code strictly speaking.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
2015-03-26 12:39 [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover Tvrtko Ursulin
2015-03-26 13:30 ` Ville Syrjälä
@ 2015-03-26 20:13 ` shuang.he
1 sibling, 0 replies; 11+ messages in thread
From: shuang.he @ 2015-03-26 20:13 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, tvrtko.ursulin
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6061
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV -1 276/276 275/276
ILK 303/303 303/303
SNB 304/304 304/304
IVB 339/339 339/339
BYT 287/287 287/287
HSW 362/362 362/362
BDW 310/310 310/310
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*PNV igt@gem_userptr_blits@minor-unsync-interruptible PASS(2) DMESG_WARN(2)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/i915_gem_evict.c:#i915_gem_evict_vm[i915]()@WARNING:.* at .* i915_gem_evict_vm+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
2015-03-26 14:05 ` Tvrtko Ursulin
@ 2015-04-13 12:09 ` Jani Nikula
2015-04-13 13:37 ` Tvrtko Ursulin
0 siblings, 1 reply; 11+ messages in thread
From: Jani Nikula @ 2015-04-13 12:09 UTC (permalink / raw)
To: Tvrtko Ursulin, Ville Syrjälä; +Cc: Intel-gfx
On Thu, 26 Mar 2015, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> On 03/26/2015 01:30 PM, Ville Syrjälä wrote:
>> On Thu, Mar 26, 2015 at 12:39:40PM +0000, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> intel_user_framebuffer_destroy() requires the struct_mutex for its
>>> object bookkeeping, so this means that all calls to
>>> drm_framebuffer_unreference must be held without that lock.
>>>
>>> This is a simplified version of the identically named patch by Chris Wilson.
>>>
>>> References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index cb50854..0788507 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -14020,11 +14020,21 @@ void intel_modeset_gem_init(struct drm_device *dev)
>>> c->primary->fb,
>>> c->primary->state,
>>> NULL)) {
>>> + /*
>>> + * We must drop struct_mutex when calling
>>> + * drm_framebuffer_unreference and it is safe to do so
>>> + * because it is not needed at this point anyway.
>>> + * At this stage the driver is still single-threaded and
>>> + * we are taking it only to silence a warning in
>>> + * intel_pin_and_fence_fb_obj.
>>> + */
>>> + mutex_unlock(&dev->struct_mutex);
>>> DRM_ERROR("failed to pin boot fb on pipe %d\n",
>>> to_intel_crtc(c)->pipe);
>>> drm_framebuffer_unreference(c->primary->fb);
>>> c->primary->fb = NULL;
>>> update_state_fb(c->primary);
>>> + mutex_lock(&dev->struct_mutex);
>>> }
>>> }
>>> mutex_unlock(&dev->struct_mutex);
>>
>> Just grab the mutex around the pin_and_fence inside the loop. It doesn't
>> protect anything else.
>
> Well the comment says so, but this way it only grabs and releases it
> once if there are multiple active crtcs and nothing fails. So I was
> hoping the comment was enough to explain the reality, even though the
> other option would be more obvious code strictly speaking.
Tvrtko & Ville, can you reach a solution on this one? Or is there a
new patch that I may have missed?
BR,
Jani.
>
> Regards,
>
> Tvrtko
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
2015-04-13 12:09 ` Jani Nikula
@ 2015-04-13 13:37 ` Tvrtko Ursulin
2015-04-13 13:52 ` Ville Syrjälä
0 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2015-04-13 13:37 UTC (permalink / raw)
To: Jani Nikula, Ville Syrjälä; +Cc: Intel-gfx
On 04/13/2015 01:09 PM, Jani Nikula wrote:
> On Thu, 26 Mar 2015, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>> On 03/26/2015 01:30 PM, Ville Syrjälä wrote:
>>> On Thu, Mar 26, 2015 at 12:39:40PM +0000, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> intel_user_framebuffer_destroy() requires the struct_mutex for its
>>>> object bookkeeping, so this means that all calls to
>>>> drm_framebuffer_unreference must be held without that lock.
>>>>
>>>> This is a simplified version of the identically named patch by Chris Wilson.
>>>>
>>>> References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
>>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
>>>> 1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>> index cb50854..0788507 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -14020,11 +14020,21 @@ void intel_modeset_gem_init(struct drm_device *dev)
>>>> c->primary->fb,
>>>> c->primary->state,
>>>> NULL)) {
>>>> + /*
>>>> + * We must drop struct_mutex when calling
>>>> + * drm_framebuffer_unreference and it is safe to do so
>>>> + * because it is not needed at this point anyway.
>>>> + * At this stage the driver is still single-threaded and
>>>> + * we are taking it only to silence a warning in
>>>> + * intel_pin_and_fence_fb_obj.
>>>> + */
>>>> + mutex_unlock(&dev->struct_mutex);
>>>> DRM_ERROR("failed to pin boot fb on pipe %d\n",
>>>> to_intel_crtc(c)->pipe);
>>>> drm_framebuffer_unreference(c->primary->fb);
>>>> c->primary->fb = NULL;
>>>> update_state_fb(c->primary);
>>>> + mutex_lock(&dev->struct_mutex);
>>>> }
>>>> }
>>>> mutex_unlock(&dev->struct_mutex);
>>>
>>> Just grab the mutex around the pin_and_fence inside the loop. It doesn't
>>> protect anything else.
>>
>> Well the comment says so, but this way it only grabs and releases it
>> once if there are multiple active crtcs and nothing fails. So I was
>> hoping the comment was enough to explain the reality, even though the
>> other option would be more obvious code strictly speaking.
>
> Tvrtko & Ville, can you reach a solution on this one? Or is there a
> new patch that I may have missed?
It was pretty much bike shedding - I am happy with this version since it
has a single lock/unlock on the normal path, compared to one pair per
active display with what Ville wanted.
Either approach makes for unclear code so needs a big comment anyway.
Which leaves only the exact placement of mutex_lock/unlock under discussion.
If we want to spend this much time on this that is.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
2015-04-13 13:37 ` Tvrtko Ursulin
@ 2015-04-13 13:52 ` Ville Syrjälä
0 siblings, 0 replies; 11+ messages in thread
From: Ville Syrjälä @ 2015-04-13 13:52 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Intel-gfx
On Mon, Apr 13, 2015 at 02:37:41PM +0100, Tvrtko Ursulin wrote:
>
> On 04/13/2015 01:09 PM, Jani Nikula wrote:
> > On Thu, 26 Mar 2015, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> >> On 03/26/2015 01:30 PM, Ville Syrjälä wrote:
> >>> On Thu, Mar 26, 2015 at 12:39:40PM +0000, Tvrtko Ursulin wrote:
> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>>
> >>>> intel_user_framebuffer_destroy() requires the struct_mutex for its
> >>>> object bookkeeping, so this means that all calls to
> >>>> drm_framebuffer_unreference must be held without that lock.
> >>>>
> >>>> This is a simplified version of the identically named patch by Chris Wilson.
> >>>>
> >>>> References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
> >>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>> ---
> >>>> drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
> >>>> 1 file changed, 10 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>>> index cb50854..0788507 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>>> @@ -14020,11 +14020,21 @@ void intel_modeset_gem_init(struct drm_device *dev)
> >>>> c->primary->fb,
> >>>> c->primary->state,
> >>>> NULL)) {
> >>>> + /*
> >>>> + * We must drop struct_mutex when calling
> >>>> + * drm_framebuffer_unreference and it is safe to do so
> >>>> + * because it is not needed at this point anyway.
> >>>> + * At this stage the driver is still single-threaded and
> >>>> + * we are taking it only to silence a warning in
> >>>> + * intel_pin_and_fence_fb_obj.
> >>>> + */
> >>>> + mutex_unlock(&dev->struct_mutex);
> >>>> DRM_ERROR("failed to pin boot fb on pipe %d\n",
> >>>> to_intel_crtc(c)->pipe);
> >>>> drm_framebuffer_unreference(c->primary->fb);
> >>>> c->primary->fb = NULL;
> >>>> update_state_fb(c->primary);
> >>>> + mutex_lock(&dev->struct_mutex);
> >>>> }
> >>>> }
> >>>> mutex_unlock(&dev->struct_mutex);
> >>>
> >>> Just grab the mutex around the pin_and_fence inside the loop. It doesn't
> >>> protect anything else.
> >>
> >> Well the comment says so, but this way it only grabs and releases it
> >> once if there are multiple active crtcs and nothing fails. So I was
> >> hoping the comment was enough to explain the reality, even though the
> >> other option would be more obvious code strictly speaking.
> >
> > Tvrtko & Ville, can you reach a solution on this one? Or is there a
> > new patch that I may have missed?
>
> It was pretty much bike shedding - I am happy with this version since it
> has a single lock/unlock on the normal path, compared to one pair per
> active display with what Ville wanted.
>
> Either approach makes for unclear code so needs a big comment anyway.
> Which leaves only the exact placement of mutex_lock/unlock under discussion.
I don't see what's unclear about locking only around the call that needs
the lock.
>
> If we want to spend this much time on this that is.
>
> Regards,
>
> Tvrtko
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
2015-04-14 9:00 ` Ville Syrjälä
@ 2015-04-14 13:19 ` Jani Nikula
0 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2015-04-14 13:19 UTC (permalink / raw)
To: Ville Syrjälä, Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx
On Tue, 14 Apr 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Apr 13, 2015 at 04:03:03PM +0100, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> intel_user_framebuffer_destroy() requires the struct_mutex for its
>> object bookkeeping, so this means that all calls to
>> drm_framebuffer_unreference must be held without that lock.
>>
>> This is a simplified version of the identically named patch by Chris Wilson.
>>
>> Regression from commit ab8d66752a9c28cd6c94fa173feacdfc1554aa03
>> Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Date: Mon Feb 2 15:44:15 2015 +0000
>>
>> drm/i915: Track old framebuffer instead of object
>>
>> v2: Bikeshedding.
>>
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Pushed to drm-intel-next-fixes, thanks for the patch and review.
BR,
Jani.
>
>> ---
>> drivers/gpu/drm/i915/intel_display.c | 14 ++++++++------
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 97922fb..5fb11bc 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -14739,6 +14739,7 @@ void intel_modeset_gem_init(struct drm_device *dev)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct drm_crtc *c;
>> struct drm_i915_gem_object *obj;
>> + int ret;
>>
>> mutex_lock(&dev->struct_mutex);
>> intel_init_gt_powersave(dev);
>> @@ -14763,16 +14764,18 @@ void intel_modeset_gem_init(struct drm_device *dev)
>> * pinned & fenced. When we do the allocation it's too early
>> * for this.
>> */
>> - mutex_lock(&dev->struct_mutex);
>> for_each_crtc(dev, c) {
>> obj = intel_fb_obj(c->primary->fb);
>> if (obj == NULL)
>> continue;
>>
>> - if (intel_pin_and_fence_fb_obj(c->primary,
>> - c->primary->fb,
>> - c->primary->state,
>> - NULL)) {
>> + mutex_lock(&dev->struct_mutex);
>> + ret = intel_pin_and_fence_fb_obj(c->primary,
>> + c->primary->fb,
>> + c->primary->state,
>> + NULL);
>> + mutex_unlock(&dev->struct_mutex);
>> + if (ret) {
>> DRM_ERROR("failed to pin boot fb on pipe %d\n",
>> to_intel_crtc(c)->pipe);
>> drm_framebuffer_unreference(c->primary->fb);
>> @@ -14780,7 +14783,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
>> update_state_fb(c->primary);
>> }
>> }
>> - mutex_unlock(&dev->struct_mutex);
>>
>> intel_backlight_register(dev);
>> }
>> --
>> 2.3.5
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
2015-04-13 15:03 Tvrtko Ursulin
2015-04-13 18:18 ` shuang.he
@ 2015-04-14 9:00 ` Ville Syrjälä
2015-04-14 13:19 ` Jani Nikula
1 sibling, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2015-04-14 9:00 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx
On Mon, Apr 13, 2015 at 04:03:03PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> intel_user_framebuffer_destroy() requires the struct_mutex for its
> object bookkeeping, so this means that all calls to
> drm_framebuffer_unreference must be held without that lock.
>
> This is a simplified version of the identically named patch by Chris Wilson.
>
> Regression from commit ab8d66752a9c28cd6c94fa173feacdfc1554aa03
> Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Date: Mon Feb 2 15:44:15 2015 +0000
>
> drm/i915: Track old framebuffer instead of object
>
> v2: Bikeshedding.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 97922fb..5fb11bc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14739,6 +14739,7 @@ void intel_modeset_gem_init(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_crtc *c;
> struct drm_i915_gem_object *obj;
> + int ret;
>
> mutex_lock(&dev->struct_mutex);
> intel_init_gt_powersave(dev);
> @@ -14763,16 +14764,18 @@ void intel_modeset_gem_init(struct drm_device *dev)
> * pinned & fenced. When we do the allocation it's too early
> * for this.
> */
> - mutex_lock(&dev->struct_mutex);
> for_each_crtc(dev, c) {
> obj = intel_fb_obj(c->primary->fb);
> if (obj == NULL)
> continue;
>
> - if (intel_pin_and_fence_fb_obj(c->primary,
> - c->primary->fb,
> - c->primary->state,
> - NULL)) {
> + mutex_lock(&dev->struct_mutex);
> + ret = intel_pin_and_fence_fb_obj(c->primary,
> + c->primary->fb,
> + c->primary->state,
> + NULL);
> + mutex_unlock(&dev->struct_mutex);
> + if (ret) {
> DRM_ERROR("failed to pin boot fb on pipe %d\n",
> to_intel_crtc(c)->pipe);
> drm_framebuffer_unreference(c->primary->fb);
> @@ -14780,7 +14783,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
> update_state_fb(c->primary);
> }
> }
> - mutex_unlock(&dev->struct_mutex);
>
> intel_backlight_register(dev);
> }
> --
> 2.3.5
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
2015-04-13 15:03 Tvrtko Ursulin
@ 2015-04-13 18:18 ` shuang.he
2015-04-14 9:00 ` Ville Syrjälä
1 sibling, 0 replies; 11+ messages in thread
From: shuang.he @ 2015-04-13 18:18 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, tvrtko.ursulin
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6186
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV -4 276/276 272/276
ILK 301/301 301/301
SNB -22 316/316 294/316
IVB -1 328/328 327/328
BYT 285/285 285/285
HSW 394/394 394/394
BDW 321/321 321/321
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
PNV igt@gem_userptr_blits@coherency-unsync CRASH(2)PASS(4) CRASH(1)PASS(1)
PNV igt@gen3_render_linear_blits FAIL(4)PASS(7) FAIL(1)PASS(1)
PNV igt@gen3_render_mixed_blits FAIL(5)PASS(6) FAIL(1)PASS(1)
PNV igt@gen3_render_tiledx_blits FAIL(5)PASS(7) FAIL(1)PASS(1)
SNB igt@kms_cursor_crc@cursor-size-change NSPT(1)PASS(1) NSPT(2)
SNB igt@kms_flip_event_leak NSPT(1)PASS(1) NSPT(2)
SNB igt@kms_mmio_vs_cs_flip@setcrtc_vs_cs_flip NSPT(2)PASS(1) NSPT(2)
SNB igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip NSPT(2)PASS(1) NSPT(2)
SNB igt@kms_rotation_crc@primary-rotation NSPT(2)PASS(1) NSPT(2)
SNB igt@kms_rotation_crc@sprite-rotation NSPT(2)PASS(3) NSPT(2)
SNB igt@pm_rpm@cursor NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@cursor-dpms NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@dpms-mode-unset-non-lpsp NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@dpms-non-lpsp NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@drm-resources-equal NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@fences NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@fences-dpms NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@gem-execbuf NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@gem-mmap-cpu NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@gem-mmap-gtt NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@gem-pread NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@i2c NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@modeset-non-lpsp NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@modeset-non-lpsp-stress-no-wait NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@pci-d3-state NSPT(2)PASS(1) NSPT(2)
SNB igt@pm_rpm@rte NSPT(2)PASS(1) NSPT(2)
IVB igt@gem_pwrite_pread@uncached-copy-performance DMESG_WARN(1)PASS(7) DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...blitter_ring_idle@Hangcheck timer elapsed... blitter ring idle
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover
@ 2015-04-13 15:03 Tvrtko Ursulin
2015-04-13 18:18 ` shuang.he
2015-04-14 9:00 ` Ville Syrjälä
0 siblings, 2 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2015-04-13 15:03 UTC (permalink / raw)
To: Intel-gfx; +Cc: Daniel Vetter
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
intel_user_framebuffer_destroy() requires the struct_mutex for its
object bookkeeping, so this means that all calls to
drm_framebuffer_unreference must be held without that lock.
This is a simplified version of the identically named patch by Chris Wilson.
Regression from commit ab8d66752a9c28cd6c94fa173feacdfc1554aa03
Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Date: Mon Feb 2 15:44:15 2015 +0000
drm/i915: Track old framebuffer instead of object
v2: Bikeshedding.
References: https://bugs.freedesktop.org/show_bug.cgi?id=89166
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 97922fb..5fb11bc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14739,6 +14739,7 @@ void intel_modeset_gem_init(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *c;
struct drm_i915_gem_object *obj;
+ int ret;
mutex_lock(&dev->struct_mutex);
intel_init_gt_powersave(dev);
@@ -14763,16 +14764,18 @@ void intel_modeset_gem_init(struct drm_device *dev)
* pinned & fenced. When we do the allocation it's too early
* for this.
*/
- mutex_lock(&dev->struct_mutex);
for_each_crtc(dev, c) {
obj = intel_fb_obj(c->primary->fb);
if (obj == NULL)
continue;
- if (intel_pin_and_fence_fb_obj(c->primary,
- c->primary->fb,
- c->primary->state,
- NULL)) {
+ mutex_lock(&dev->struct_mutex);
+ ret = intel_pin_and_fence_fb_obj(c->primary,
+ c->primary->fb,
+ c->primary->state,
+ NULL);
+ mutex_unlock(&dev->struct_mutex);
+ if (ret) {
DRM_ERROR("failed to pin boot fb on pipe %d\n",
to_intel_crtc(c)->pipe);
drm_framebuffer_unreference(c->primary->fb);
@@ -14780,7 +14783,6 @@ void intel_modeset_gem_init(struct drm_device *dev)
update_state_fb(c->primary);
}
}
- mutex_unlock(&dev->struct_mutex);
intel_backlight_register(dev);
}
--
2.3.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-04-14 12:18 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-26 12:39 [PATCH] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for takeover Tvrtko Ursulin
2015-03-26 13:30 ` Ville Syrjälä
2015-03-26 14:05 ` Tvrtko Ursulin
2015-04-13 12:09 ` Jani Nikula
2015-04-13 13:37 ` Tvrtko Ursulin
2015-04-13 13:52 ` Ville Syrjälä
2015-03-26 20:13 ` shuang.he
2015-04-13 15:03 Tvrtko Ursulin
2015-04-13 18:18 ` shuang.he
2015-04-14 9:00 ` Ville Syrjälä
2015-04-14 13:19 ` Jani Nikula
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.