* [PATCH] drm/i915: Hold runtime PM during plane commit
@ 2014-12-15 18:11 Matt Roper
2014-12-15 18:30 ` Paulo Zanoni
2014-12-16 0:57 ` shuang.he
0 siblings, 2 replies; 8+ messages in thread
From: Matt Roper @ 2014-12-15 18:11 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
During plane operations, we read/write some registers that only operate
properly if we're not runtime suspended. At the moment we're not
holding the runtime PM reference across the whole plane operation, so
there's a potential for problems.
This issue was already partially addressed by commit
commit d6dd6843ff4a57c662dbc378b9f99a9c034b0956
Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
Date: Fri Aug 15 15:59:32 2014 -0300
drm/i915: fix plane/cursor handling when runtime suspended
which took care of holding the runtime PM reference during the pin and
fence operations for plane updates. However there are still a few
actual plane registers that we also need to hold the runtime PM
reference for. Recent refactoring patches in preparation for atomic
have rearranged the code and made it increasingly likely that the
hardware will have time to suspend between the pin/fence operation and
the actual register writes.
The solution here grabs the runtime PM reference around the 'commit'
operation for planes, which should cover all the relevant register
reads/writes.
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87180
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3044af5..a0ddce5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11863,6 +11863,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_w, uint32_t src_h)
{
struct drm_device *dev = plane->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_framebuffer *old_fb = plane->fb;
struct intel_plane_state state;
struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -11902,7 +11903,9 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
return ret;
}
+ intel_runtime_pm_get(dev_priv);
intel_plane->commit_plane(plane, &state);
+ intel_runtime_pm_put(dev_priv);
if (fb != old_fb && old_fb) {
if (intel_crtc->active)
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Hold runtime PM during plane commit
2014-12-15 18:11 [PATCH] drm/i915: Hold runtime PM during plane commit Matt Roper
@ 2014-12-15 18:30 ` Paulo Zanoni
2014-12-16 9:42 ` Daniel Vetter
2014-12-16 0:57 ` shuang.he
1 sibling, 1 reply; 8+ messages in thread
From: Paulo Zanoni @ 2014-12-15 18:30 UTC (permalink / raw)
To: Matt Roper; +Cc: Intel Graphics Development, Paulo Zanoni
2014-12-15 16:11 GMT-02:00 Matt Roper <matthew.d.roper@intel.com>:
> During plane operations, we read/write some registers that only operate
> properly if we're not runtime suspended. At the moment we're not
> holding the runtime PM reference across the whole plane operation, so
> there's a potential for problems.
>
> This issue was already partially addressed by commit
>
> commit d6dd6843ff4a57c662dbc378b9f99a9c034b0956
> Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Date: Fri Aug 15 15:59:32 2014 -0300
>
> drm/i915: fix plane/cursor handling when runtime suspended
>
> which took care of holding the runtime PM reference during the pin and
> fence operations for plane updates. However there are still a few
> actual plane registers that we also need to hold the runtime PM
> reference for. Recent refactoring patches in preparation for atomic
> have rearranged the code and made it increasingly likely that the
> hardware will have time to suspend between the pin/fence operation and
> the actual register writes.
>
> The solution here grabs the runtime PM reference around the 'commit'
> operation for planes, which should cover all the relevant register
> reads/writes.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87180
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
I see we're in the middle of a very big rework on how all these
prepare/commit functions are called, so I don't think it makes sense
to spend too much time trying to find the very-best-perfect spot for
the get/put calls, since they're likely to be changed later. So I
guess that for now it's important to fix the current "regression"
reported by QA:
Testcase: igt/pm-rpm/legacy-planes
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3044af5..a0ddce5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11863,6 +11863,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> uint32_t src_w, uint32_t src_h)
> {
> struct drm_device *dev = plane->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_framebuffer *old_fb = plane->fb;
> struct intel_plane_state state;
> struct intel_plane *intel_plane = to_intel_plane(plane);
> @@ -11902,7 +11903,9 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> return ret;
> }
>
> + intel_runtime_pm_get(dev_priv);
> intel_plane->commit_plane(plane, &state);
> + intel_runtime_pm_put(dev_priv);
>
> if (fb != old_fb && old_fb) {
> if (intel_crtc->active)
> --
> 1.8.5.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Hold runtime PM during plane commit
2014-12-15 18:11 [PATCH] drm/i915: Hold runtime PM during plane commit Matt Roper
2014-12-15 18:30 ` Paulo Zanoni
@ 2014-12-16 0:57 ` shuang.he
1 sibling, 0 replies; 8+ messages in thread
From: shuang.he @ 2014-12-16 0:57 UTC (permalink / raw)
To: shuang.he, intel-gfx, matthew.d.roper
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 364/364 364/364
ILK +5 360/366 365/366
SNB 448/450 448/450
IVB 497/498 497/498
BYT 289/289 289/289
HSW 563/564 563/564
BDW 417/417 417/417
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
ILK igt_drv_suspend_fence-restore-untiled DMESG_WARN(1, M26)PASS(5, M37M26) PASS(1, M37)
ILK igt_kms_flip_bcs-flip-vs-modeset-interruptible DMESG_WARN(1, M26)PASS(5, M37M26) PASS(1, M37)
ILK igt_kms_flip_busy-flip-interruptible DMESG_WARN(1, M26)PASS(5, M37M26) PASS(1, M37)
ILK igt_kms_flip_flip-vs-rmfb-interruptible DMESG_WARN(1, M26)PASS(5, M37M26) PASS(1, M37)
ILK igt_kms_flip_rcs-flip-vs-dpms DMESG_WARN(1, M26)PASS(4, M37M26) PASS(1, M37)
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] 8+ messages in thread
* Re: [PATCH] drm/i915: Hold runtime PM during plane commit
2014-12-15 18:30 ` Paulo Zanoni
@ 2014-12-16 9:42 ` Daniel Vetter
2014-12-16 9:59 ` Jani Nikula
2014-12-17 19:10 ` Paulo Zanoni
0 siblings, 2 replies; 8+ messages in thread
From: Daniel Vetter @ 2014-12-16 9:42 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, Paulo Zanoni
On Mon, Dec 15, 2014 at 04:30:44PM -0200, Paulo Zanoni wrote:
> 2014-12-15 16:11 GMT-02:00 Matt Roper <matthew.d.roper@intel.com>:
> > During plane operations, we read/write some registers that only operate
> > properly if we're not runtime suspended. At the moment we're not
> > holding the runtime PM reference across the whole plane operation, so
> > there's a potential for problems.
> >
> > This issue was already partially addressed by commit
> >
> > commit d6dd6843ff4a57c662dbc378b9f99a9c034b0956
> > Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Date: Fri Aug 15 15:59:32 2014 -0300
> >
> > drm/i915: fix plane/cursor handling when runtime suspended
> >
> > which took care of holding the runtime PM reference during the pin and
> > fence operations for plane updates. However there are still a few
> > actual plane registers that we also need to hold the runtime PM
> > reference for. Recent refactoring patches in preparation for atomic
> > have rearranged the code and made it increasingly likely that the
> > hardware will have time to suspend between the pin/fence operation and
> > the actual register writes.
Which kind of registers? If this is just in the system agent then a rpm
ref is enough, but if this is also about plane registers then we'd need a
reference of the plane power domain. Which would indicate some failure to
check for crtc->active somewhere I think.
> > The solution here grabs the runtime PM reference around the 'commit'
> > operation for planes, which should cover all the relevant register
> > reads/writes.
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87180
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>
> I see we're in the middle of a very big rework on how all these
> prepare/commit functions are called, so I don't think it makes sense
> to spend too much time trying to find the very-best-perfect spot for
> the get/put calls, since they're likely to be changed later. So I
> guess that for now it's important to fix the current "regression"
> reported by QA:
>
> Testcase: igt/pm-rpm/legacy-planes
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Also Cc: stable@vger.kernel.org I guess? Or is this only for dinq?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Hold runtime PM during plane commit
2014-12-16 9:42 ` Daniel Vetter
@ 2014-12-16 9:59 ` Jani Nikula
2014-12-16 13:29 ` Jani Nikula
2014-12-17 19:10 ` Paulo Zanoni
1 sibling, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2014-12-16 9:59 UTC (permalink / raw)
To: Daniel Vetter, Paulo Zanoni; +Cc: Intel Graphics Development, Paulo Zanoni
On Tue, 16 Dec 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Dec 15, 2014 at 04:30:44PM -0200, Paulo Zanoni wrote:
>> 2014-12-15 16:11 GMT-02:00 Matt Roper <matthew.d.roper@intel.com>:
>> > During plane operations, we read/write some registers that only operate
>> > properly if we're not runtime suspended. At the moment we're not
>> > holding the runtime PM reference across the whole plane operation, so
>> > there's a potential for problems.
>> >
>> > This issue was already partially addressed by commit
>> >
>> > commit d6dd6843ff4a57c662dbc378b9f99a9c034b0956
>> > Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > Date: Fri Aug 15 15:59:32 2014 -0300
>> >
>> > drm/i915: fix plane/cursor handling when runtime suspended
>> >
>> > which took care of holding the runtime PM reference during the pin and
>> > fence operations for plane updates. However there are still a few
>> > actual plane registers that we also need to hold the runtime PM
>> > reference for. Recent refactoring patches in preparation for atomic
>> > have rearranged the code and made it increasingly likely that the
>> > hardware will have time to suspend between the pin/fence operation and
>> > the actual register writes.
>
> Which kind of registers? If this is just in the system agent then a rpm
> ref is enough, but if this is also about plane registers then we'd need a
> reference of the plane power domain. Which would indicate some failure to
> check for crtc->active somewhere I think.
>
>> > The solution here grabs the runtime PM reference around the 'commit'
>> > operation for planes, which should cover all the relevant register
>> > reads/writes.
>> >
>> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87180
>> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>>
>> I see we're in the middle of a very big rework on how all these
>> prepare/commit functions are called, so I don't think it makes sense
>> to spend too much time trying to find the very-best-perfect spot for
>> the get/put calls, since they're likely to be changed later. So I
>> guess that for now it's important to fix the current "regression"
>> reported by QA:
>>
>> Testcase: igt/pm-rpm/legacy-planes
>> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Also Cc: stable@vger.kernel.org I guess? Or is this only for dinq?
At least the partial fix referenced is in 3.17.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> 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] 8+ messages in thread
* Re: [PATCH] drm/i915: Hold runtime PM during plane commit
2014-12-16 9:59 ` Jani Nikula
@ 2014-12-16 13:29 ` Jani Nikula
2014-12-17 20:35 ` Daniel Vetter
0 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2014-12-16 13:29 UTC (permalink / raw)
To: Daniel Vetter, Paulo Zanoni; +Cc: Intel Graphics Development, Paulo Zanoni
On Tue, 16 Dec 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Tue, 16 Dec 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Mon, Dec 15, 2014 at 04:30:44PM -0200, Paulo Zanoni wrote:
>>> 2014-12-15 16:11 GMT-02:00 Matt Roper <matthew.d.roper@intel.com>:
>>> > During plane operations, we read/write some registers that only operate
>>> > properly if we're not runtime suspended. At the moment we're not
>>> > holding the runtime PM reference across the whole plane operation, so
>>> > there's a potential for problems.
>>> >
>>> > This issue was already partially addressed by commit
>>> >
>>> > commit d6dd6843ff4a57c662dbc378b9f99a9c034b0956
>>> > Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>> > Date: Fri Aug 15 15:59:32 2014 -0300
>>> >
>>> > drm/i915: fix plane/cursor handling when runtime suspended
>>> >
>>> > which took care of holding the runtime PM reference during the pin and
>>> > fence operations for plane updates. However there are still a few
>>> > actual plane registers that we also need to hold the runtime PM
>>> > reference for. Recent refactoring patches in preparation for atomic
>>> > have rearranged the code and made it increasingly likely that the
>>> > hardware will have time to suspend between the pin/fence operation and
>>> > the actual register writes.
>>
>> Which kind of registers? If this is just in the system agent then a rpm
>> ref is enough, but if this is also about plane registers then we'd need a
>> reference of the plane power domain. Which would indicate some failure to
>> check for crtc->active somewhere I think.
>>
>>> > The solution here grabs the runtime PM reference around the 'commit'
>>> > operation for planes, which should cover all the relevant register
>>> > reads/writes.
>>> >
>>> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87180
>>> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>>>
>>> I see we're in the middle of a very big rework on how all these
>>> prepare/commit functions are called, so I don't think it makes sense
>>> to spend too much time trying to find the very-best-perfect spot for
>>> the get/put calls, since they're likely to be changed later. So I
>>> guess that for now it's important to fix the current "regression"
>>> reported by QA:
>>>
>>> Testcase: igt/pm-rpm/legacy-planes
>>> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> Also Cc: stable@vger.kernel.org I guess? Or is this only for dinq?
>
> At least the partial fix referenced is in 3.17.
Ugh, but it conflicts badly, there's no ->commit_plane in
drm-intel-next-fixes.
Daniel, pick this up for dinq, and if someone wants this to stable, it
needs a backported version.
BR,
Jani.
>
>> -Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
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] 8+ messages in thread
* Re: [PATCH] drm/i915: Hold runtime PM during plane commit
2014-12-16 9:42 ` Daniel Vetter
2014-12-16 9:59 ` Jani Nikula
@ 2014-12-17 19:10 ` Paulo Zanoni
1 sibling, 0 replies; 8+ messages in thread
From: Paulo Zanoni @ 2014-12-17 19:10 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development, Paulo Zanoni
2014-12-16 7:42 GMT-02:00 Daniel Vetter <daniel@ffwll.ch>:
> On Mon, Dec 15, 2014 at 04:30:44PM -0200, Paulo Zanoni wrote:
>> 2014-12-15 16:11 GMT-02:00 Matt Roper <matthew.d.roper@intel.com>:
>> > During plane operations, we read/write some registers that only operate
>> > properly if we're not runtime suspended. At the moment we're not
>> > holding the runtime PM reference across the whole plane operation, so
>> > there's a potential for problems.
>> >
>> > This issue was already partially addressed by commit
>> >
>> > commit d6dd6843ff4a57c662dbc378b9f99a9c034b0956
>> > Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > Date: Fri Aug 15 15:59:32 2014 -0300
>> >
>> > drm/i915: fix plane/cursor handling when runtime suspended
>> >
>> > which took care of holding the runtime PM reference during the pin and
>> > fence operations for plane updates. However there are still a few
>> > actual plane registers that we also need to hold the runtime PM
>> > reference for. Recent refactoring patches in preparation for atomic
>> > have rearranged the code and made it increasingly likely that the
>> > hardware will have time to suspend between the pin/fence operation and
>> > the actual register writes.
>
> Which kind of registers?
Some of the regs touched by ivb_get_colorkey().
> If this is just in the system agent then a rpm
> ref is enough, but if this is also about plane registers then we'd need a
> reference of the plane power domain. Which would indicate some failure to
> check for crtc->active somewhere I think.
>
>> > The solution here grabs the runtime PM reference around the 'commit'
>> > operation for planes, which should cover all the relevant register
>> > reads/writes.
>> >
>> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87180
>> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>>
>> I see we're in the middle of a very big rework on how all these
>> prepare/commit functions are called, so I don't think it makes sense
>> to spend too much time trying to find the very-best-perfect spot for
>> the get/put calls, since they're likely to be changed later. So I
>> guess that for now it's important to fix the current "regression"
>> reported by QA:
>>
>> Testcase: igt/pm-rpm/legacy-planes
>> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Also Cc: stable@vger.kernel.org I guess? Or is this only for dinq?
I would suggest only dinq, since it's the only thing that can
reproduce the WARN, which got visible after:
commit 6beb8c23ebcc3d3287d8a247d11b73d7d0eaa475
drm/i915: Consolidate plane 'prepare' functions (v2)
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Hold runtime PM during plane commit
2014-12-16 13:29 ` Jani Nikula
@ 2014-12-17 20:35 ` Daniel Vetter
0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2014-12-17 20:35 UTC (permalink / raw)
To: Jani Nikula; +Cc: Intel Graphics Development, Paulo Zanoni
On Tue, Dec 16, 2014 at 03:29:56PM +0200, Jani Nikula wrote:
> On Tue, 16 Dec 2014, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > On Tue, 16 Dec 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> >> On Mon, Dec 15, 2014 at 04:30:44PM -0200, Paulo Zanoni wrote:
> >>> 2014-12-15 16:11 GMT-02:00 Matt Roper <matthew.d.roper@intel.com>:
> >>> > During plane operations, we read/write some registers that only operate
> >>> > properly if we're not runtime suspended. At the moment we're not
> >>> > holding the runtime PM reference across the whole plane operation, so
> >>> > there's a potential for problems.
> >>> >
> >>> > This issue was already partially addressed by commit
> >>> >
> >>> > commit d6dd6843ff4a57c662dbc378b9f99a9c034b0956
> >>> > Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>> > Date: Fri Aug 15 15:59:32 2014 -0300
> >>> >
> >>> > drm/i915: fix plane/cursor handling when runtime suspended
> >>> >
> >>> > which took care of holding the runtime PM reference during the pin and
> >>> > fence operations for plane updates. However there are still a few
> >>> > actual plane registers that we also need to hold the runtime PM
> >>> > reference for. Recent refactoring patches in preparation for atomic
> >>> > have rearranged the code and made it increasingly likely that the
> >>> > hardware will have time to suspend between the pin/fence operation and
> >>> > the actual register writes.
> >>
> >> Which kind of registers? If this is just in the system agent then a rpm
> >> ref is enough, but if this is also about plane registers then we'd need a
> >> reference of the plane power domain. Which would indicate some failure to
> >> check for crtc->active somewhere I think.
> >>
> >>> > The solution here grabs the runtime PM reference around the 'commit'
> >>> > operation for planes, which should cover all the relevant register
> >>> > reads/writes.
> >>> >
> >>> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87180
> >>> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> >>>
> >>> I see we're in the middle of a very big rework on how all these
> >>> prepare/commit functions are called, so I don't think it makes sense
> >>> to spend too much time trying to find the very-best-perfect spot for
> >>> the get/put calls, since they're likely to be changed later. So I
> >>> guess that for now it's important to fix the current "regression"
> >>> reported by QA:
> >>>
> >>> Testcase: igt/pm-rpm/legacy-planes
> >>> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> Also Cc: stable@vger.kernel.org I guess? Or is this only for dinq?
> >
> > At least the partial fix referenced is in 3.17.
>
> Ugh, but it conflicts badly, there's no ->commit_plane in
> drm-intel-next-fixes.
>
> Daniel, pick this up for dinq, and if someone wants this to stable, it
> needs a backported version.
Done, and thanks to Paulo for the additional clarification. I've augmented
the commit message with that.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-12-17 20:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-15 18:11 [PATCH] drm/i915: Hold runtime PM during plane commit Matt Roper
2014-12-15 18:30 ` Paulo Zanoni
2014-12-16 9:42 ` Daniel Vetter
2014-12-16 9:59 ` Jani Nikula
2014-12-16 13:29 ` Jani Nikula
2014-12-17 20:35 ` Daniel Vetter
2014-12-17 19:10 ` Paulo Zanoni
2014-12-16 0:57 ` shuang.he
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.