* [PATCH 1/2] drm/i915: Clear self-refresh watermarks when disabled
@ 2012-12-07 10:43 Chris Wilson
2012-12-07 10:43 ` [PATCH 2/2] drm/i915: Prefer CRTC 'active' rather than 'enabled' during WM computations Chris Wilson
2012-12-10 20:57 ` [PATCH 1/2] drm/i915: Clear self-refresh watermarks when disabled Jesse Barnes
0 siblings, 2 replies; 5+ messages in thread
From: Chris Wilson @ 2012-12-07 10:43 UTC (permalink / raw)
To: intel-gfx
If we elect to disable self-refresh as they require too many FIFO
entries, clear the values prior to writing them into the registers. If
they are too large they may occupy more bits than available and so
corrupt neighbouring WM values.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f6b4697..3eb21fa 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1306,7 +1306,6 @@ static void valleyview_update_wm(struct drm_device *dev)
&planeb_wm, &cursorb_wm))
enabled |= 2;
- plane_sr = cursor_sr = 0;
if (single_plane_enabled(enabled) &&
g4x_compute_srwm(dev, ffs(enabled) - 1,
sr_latency_ns,
@@ -1317,11 +1316,13 @@ static void valleyview_update_wm(struct drm_device *dev)
2*sr_latency_ns,
&valleyview_wm_info,
&valleyview_cursor_wm_info,
- &ignore_plane_sr, &cursor_sr))
+ &ignore_plane_sr, &cursor_sr)) {
I915_WRITE(FW_BLC_SELF_VLV, FW_CSPWRDWNEN);
- else
+ } else {
I915_WRITE(FW_BLC_SELF_VLV,
I915_READ(FW_BLC_SELF_VLV) & ~FW_CSPWRDWNEN);
+ plane_sr = cursor_sr = 0;
+ }
DRM_DEBUG_KMS("Setting FIFO watermarks - A: plane=%d, cursor=%d, B: plane=%d, cursor=%d, SR: plane=%d, cursor=%d\n",
planea_wm, cursora_wm,
@@ -1361,17 +1362,18 @@ static void g4x_update_wm(struct drm_device *dev)
&planeb_wm, &cursorb_wm))
enabled |= 2;
- plane_sr = cursor_sr = 0;
if (single_plane_enabled(enabled) &&
g4x_compute_srwm(dev, ffs(enabled) - 1,
sr_latency_ns,
&g4x_wm_info,
&g4x_cursor_wm_info,
- &plane_sr, &cursor_sr))
+ &plane_sr, &cursor_sr)) {
I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN);
- else
+ } else {
I915_WRITE(FW_BLC_SELF,
I915_READ(FW_BLC_SELF) & ~FW_BLC_SELF_EN);
+ plane_sr = cursor_sr = 0;
+ }
DRM_DEBUG_KMS("Setting FIFO watermarks - A: plane=%d, cursor=%d, B: plane=%d, cursor=%d, SR: plane=%d, cursor=%d\n",
planea_wm, cursora_wm,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] drm/i915: Prefer CRTC 'active' rather than 'enabled' during WM computations
2012-12-07 10:43 [PATCH 1/2] drm/i915: Clear self-refresh watermarks when disabled Chris Wilson
@ 2012-12-07 10:43 ` Chris Wilson
2012-12-10 20:58 ` Jesse Barnes
2012-12-10 20:57 ` [PATCH 1/2] drm/i915: Clear self-refresh watermarks when disabled Jesse Barnes
1 sibling, 1 reply; 5+ messages in thread
From: Chris Wilson @ 2012-12-07 10:43 UTC (permalink / raw)
To: intel-gfx
Only the intel_crtc->active is accurate at the point where we wish to
perform WM computations, so use it instead of crtc->enabled.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_pm.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3eb21fa..07e0a2f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -405,7 +405,7 @@ void intel_update_fbc(struct drm_device *dev)
* - going to an unsupported config (interlace, pixel multiply, etc.)
*/
list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, head) {
- if (tmp_crtc->enabled &&
+ if (to_intel_crtc(tmp_crtc)->active &&
!to_intel_crtc(tmp_crtc)->primary_disabled &&
tmp_crtc->fb) {
if (crtc) {
@@ -995,7 +995,7 @@ static struct drm_crtc *single_enabled_crtc(struct drm_device *dev)
struct drm_crtc *crtc, *enabled = NULL;
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
- if (crtc->enabled && crtc->fb) {
+ if (to_intel_crtc(crtc)->active && crtc->fb) {
if (enabled)
return NULL;
enabled = crtc;
@@ -1089,7 +1089,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
int entries, tlb_miss;
crtc = intel_get_crtc_for_plane(dev, plane);
- if (crtc->fb == NULL || !crtc->enabled) {
+ if (crtc->fb == NULL || !to_intel_crtc(crtc)->active) {
*cursor_wm = cursor->guard_size;
*plane_wm = display->guard_size;
return false;
@@ -1218,7 +1218,7 @@ static bool vlv_compute_drain_latency(struct drm_device *dev,
int entries;
crtc = intel_get_crtc_for_plane(dev, plane);
- if (crtc->fb == NULL || !crtc->enabled)
+ if (crtc->fb == NULL || !to_intel_crtc(crtc)->active)
return false;
clock = crtc->mode.clock; /* VESA DOT Clock */
@@ -1479,7 +1479,7 @@ static void i9xx_update_wm(struct drm_device *dev)
fifo_size = dev_priv->display.get_fifo_size(dev, 0);
crtc = intel_get_crtc_for_plane(dev, 0);
- if (crtc->enabled && crtc->fb) {
+ if (to_intel_crtc(crtc)->active && crtc->fb) {
int cpp = crtc->fb->bits_per_pixel / 8;
if (IS_GEN2(dev))
cpp = 4;
@@ -1493,7 +1493,7 @@ static void i9xx_update_wm(struct drm_device *dev)
fifo_size = dev_priv->display.get_fifo_size(dev, 1);
crtc = intel_get_crtc_for_plane(dev, 1);
- if (crtc->enabled && crtc->fb) {
+ if (to_intel_crtc(crtc)->active && crtc->fb) {
int cpp = crtc->fb->bits_per_pixel / 8;
if (IS_GEN2(dev))
cpp = 4;
@@ -2047,7 +2047,7 @@ sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
int entries, tlb_miss;
crtc = intel_get_crtc_for_plane(dev, plane);
- if (crtc->fb == NULL || !crtc->enabled) {
+ if (crtc->fb == NULL || !to_intel_crtc(crtc)->active) {
*sprite_wm = display->guard_size;
return false;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] drm/i915: Clear self-refresh watermarks when disabled
2012-12-07 10:43 [PATCH 1/2] drm/i915: Clear self-refresh watermarks when disabled Chris Wilson
2012-12-07 10:43 ` [PATCH 2/2] drm/i915: Prefer CRTC 'active' rather than 'enabled' during WM computations Chris Wilson
@ 2012-12-10 20:57 ` Jesse Barnes
1 sibling, 0 replies; 5+ messages in thread
From: Jesse Barnes @ 2012-12-10 20:57 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Fri, 7 Dec 2012 10:43:24 +0000
Chris Wilson <chris@chris-wilson.co.uk> wrote:
> If we elect to disable self-refresh as they require too many FIFO
> entries, clear the values prior to writing them into the registers. If
> they are too large they may occupy more bits than available and so
> corrupt neighbouring WM values.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f6b4697..3eb21fa 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1306,7 +1306,6 @@ static void valleyview_update_wm(struct drm_device *dev)
> &planeb_wm, &cursorb_wm))
> enabled |= 2;
>
> - plane_sr = cursor_sr = 0;
> if (single_plane_enabled(enabled) &&
> g4x_compute_srwm(dev, ffs(enabled) - 1,
> sr_latency_ns,
> @@ -1317,11 +1316,13 @@ static void valleyview_update_wm(struct drm_device *dev)
> 2*sr_latency_ns,
> &valleyview_wm_info,
> &valleyview_cursor_wm_info,
> - &ignore_plane_sr, &cursor_sr))
> + &ignore_plane_sr, &cursor_sr)) {
> I915_WRITE(FW_BLC_SELF_VLV, FW_CSPWRDWNEN);
> - else
> + } else {
> I915_WRITE(FW_BLC_SELF_VLV,
> I915_READ(FW_BLC_SELF_VLV) & ~FW_CSPWRDWNEN);
> + plane_sr = cursor_sr = 0;
> + }
>
> DRM_DEBUG_KMS("Setting FIFO watermarks - A: plane=%d, cursor=%d, B: plane=%d, cursor=%d, SR: plane=%d, cursor=%d\n",
> planea_wm, cursora_wm,
> @@ -1361,17 +1362,18 @@ static void g4x_update_wm(struct drm_device *dev)
> &planeb_wm, &cursorb_wm))
> enabled |= 2;
>
> - plane_sr = cursor_sr = 0;
> if (single_plane_enabled(enabled) &&
> g4x_compute_srwm(dev, ffs(enabled) - 1,
> sr_latency_ns,
> &g4x_wm_info,
> &g4x_cursor_wm_info,
> - &plane_sr, &cursor_sr))
> + &plane_sr, &cursor_sr)) {
> I915_WRITE(FW_BLC_SELF, FW_BLC_SELF_EN);
> - else
> + } else {
> I915_WRITE(FW_BLC_SELF,
> I915_READ(FW_BLC_SELF) & ~FW_BLC_SELF_EN);
> + plane_sr = cursor_sr = 0;
> + }
>
> DRM_DEBUG_KMS("Setting FIFO watermarks - A: plane=%d, cursor=%d, B: plane=%d, cursor=%d, SR: plane=%d, cursor=%d\n",
> planea_wm, cursora_wm,
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/i915: Prefer CRTC 'active' rather than 'enabled' during WM computations
2012-12-07 10:43 ` [PATCH 2/2] drm/i915: Prefer CRTC 'active' rather than 'enabled' during WM computations Chris Wilson
@ 2012-12-10 20:58 ` Jesse Barnes
2012-12-17 11:39 ` Daniel Vetter
0 siblings, 1 reply; 5+ messages in thread
From: Jesse Barnes @ 2012-12-10 20:58 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Fri, 7 Dec 2012 10:43:25 +0000
Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Only the intel_crtc->active is accurate at the point where we wish to
> perform WM computations, so use it instead of crtc->enabled.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3eb21fa..07e0a2f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -405,7 +405,7 @@ void intel_update_fbc(struct drm_device *dev)
> * - going to an unsupported config (interlace, pixel multiply, etc.)
> */
> list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, head) {
> - if (tmp_crtc->enabled &&
> + if (to_intel_crtc(tmp_crtc)->active &&
> !to_intel_crtc(tmp_crtc)->primary_disabled &&
> tmp_crtc->fb) {
> if (crtc) {
> @@ -995,7 +995,7 @@ static struct drm_crtc *single_enabled_crtc(struct drm_device *dev)
> struct drm_crtc *crtc, *enabled = NULL;
>
> list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> - if (crtc->enabled && crtc->fb) {
> + if (to_intel_crtc(crtc)->active && crtc->fb) {
> if (enabled)
> return NULL;
> enabled = crtc;
> @@ -1089,7 +1089,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
> int entries, tlb_miss;
>
> crtc = intel_get_crtc_for_plane(dev, plane);
> - if (crtc->fb == NULL || !crtc->enabled) {
> + if (crtc->fb == NULL || !to_intel_crtc(crtc)->active) {
> *cursor_wm = cursor->guard_size;
> *plane_wm = display->guard_size;
> return false;
> @@ -1218,7 +1218,7 @@ static bool vlv_compute_drain_latency(struct drm_device *dev,
> int entries;
>
> crtc = intel_get_crtc_for_plane(dev, plane);
> - if (crtc->fb == NULL || !crtc->enabled)
> + if (crtc->fb == NULL || !to_intel_crtc(crtc)->active)
> return false;
>
> clock = crtc->mode.clock; /* VESA DOT Clock */
> @@ -1479,7 +1479,7 @@ static void i9xx_update_wm(struct drm_device *dev)
>
> fifo_size = dev_priv->display.get_fifo_size(dev, 0);
> crtc = intel_get_crtc_for_plane(dev, 0);
> - if (crtc->enabled && crtc->fb) {
> + if (to_intel_crtc(crtc)->active && crtc->fb) {
> int cpp = crtc->fb->bits_per_pixel / 8;
> if (IS_GEN2(dev))
> cpp = 4;
> @@ -1493,7 +1493,7 @@ static void i9xx_update_wm(struct drm_device *dev)
>
> fifo_size = dev_priv->display.get_fifo_size(dev, 1);
> crtc = intel_get_crtc_for_plane(dev, 1);
> - if (crtc->enabled && crtc->fb) {
> + if (to_intel_crtc(crtc)->active && crtc->fb) {
> int cpp = crtc->fb->bits_per_pixel / 8;
> if (IS_GEN2(dev))
> cpp = 4;
> @@ -2047,7 +2047,7 @@ sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
> int entries, tlb_miss;
>
> crtc = intel_get_crtc_for_plane(dev, plane);
> - if (crtc->fb == NULL || !crtc->enabled) {
> + if (crtc->fb == NULL || !to_intel_crtc(crtc)->active) {
> *sprite_wm = display->guard_size;
> return false;
> }
Our wm code was never very pretty since it didn't fit in very well with
the modesetting code structure... can we make it cleaner now and remove
all this duplication?
Definitely not something for this patch, just an observation.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/i915: Prefer CRTC 'active' rather than 'enabled' during WM computations
2012-12-10 20:58 ` Jesse Barnes
@ 2012-12-17 11:39 ` Daniel Vetter
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2012-12-17 11:39 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx
On Mon, Dec 10, 2012 at 12:58:40PM -0800, Jesse Barnes wrote:
> On Fri, 7 Dec 2012 10:43:25 +0000
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> > Only the intel_crtc->active is accurate at the point where we wish to
> > perform WM computations, so use it instead of crtc->enabled.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 3eb21fa..07e0a2f 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -405,7 +405,7 @@ void intel_update_fbc(struct drm_device *dev)
> > * - going to an unsupported config (interlace, pixel multiply, etc.)
> > */
> > list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, head) {
> > - if (tmp_crtc->enabled &&
> > + if (to_intel_crtc(tmp_crtc)->active &&
> > !to_intel_crtc(tmp_crtc)->primary_disabled &&
> > tmp_crtc->fb) {
> > if (crtc) {
> > @@ -995,7 +995,7 @@ static struct drm_crtc *single_enabled_crtc(struct drm_device *dev)
> > struct drm_crtc *crtc, *enabled = NULL;
> >
> > list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> > - if (crtc->enabled && crtc->fb) {
> > + if (to_intel_crtc(crtc)->active && crtc->fb) {
> > if (enabled)
> > return NULL;
> > enabled = crtc;
> > @@ -1089,7 +1089,7 @@ static bool g4x_compute_wm0(struct drm_device *dev,
> > int entries, tlb_miss;
> >
> > crtc = intel_get_crtc_for_plane(dev, plane);
> > - if (crtc->fb == NULL || !crtc->enabled) {
> > + if (crtc->fb == NULL || !to_intel_crtc(crtc)->active) {
> > *cursor_wm = cursor->guard_size;
> > *plane_wm = display->guard_size;
> > return false;
> > @@ -1218,7 +1218,7 @@ static bool vlv_compute_drain_latency(struct drm_device *dev,
> > int entries;
> >
> > crtc = intel_get_crtc_for_plane(dev, plane);
> > - if (crtc->fb == NULL || !crtc->enabled)
> > + if (crtc->fb == NULL || !to_intel_crtc(crtc)->active)
> > return false;
> >
> > clock = crtc->mode.clock; /* VESA DOT Clock */
> > @@ -1479,7 +1479,7 @@ static void i9xx_update_wm(struct drm_device *dev)
> >
> > fifo_size = dev_priv->display.get_fifo_size(dev, 0);
> > crtc = intel_get_crtc_for_plane(dev, 0);
> > - if (crtc->enabled && crtc->fb) {
> > + if (to_intel_crtc(crtc)->active && crtc->fb) {
> > int cpp = crtc->fb->bits_per_pixel / 8;
> > if (IS_GEN2(dev))
> > cpp = 4;
> > @@ -1493,7 +1493,7 @@ static void i9xx_update_wm(struct drm_device *dev)
> >
> > fifo_size = dev_priv->display.get_fifo_size(dev, 1);
> > crtc = intel_get_crtc_for_plane(dev, 1);
> > - if (crtc->enabled && crtc->fb) {
> > + if (to_intel_crtc(crtc)->active && crtc->fb) {
> > int cpp = crtc->fb->bits_per_pixel / 8;
> > if (IS_GEN2(dev))
> > cpp = 4;
> > @@ -2047,7 +2047,7 @@ sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
> > int entries, tlb_miss;
> >
> > crtc = intel_get_crtc_for_plane(dev, plane);
> > - if (crtc->fb == NULL || !crtc->enabled) {
> > + if (crtc->fb == NULL || !to_intel_crtc(crtc)->active) {
> > *sprite_wm = display->guard_size;
> > return false;
> > }
>
> Our wm code was never very pretty since it didn't fit in very well with
> the modesetting code structure... can we make it cleaner now and remove
> all this duplication?
>
> Definitely not something for this patch, just an observation.
>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Both patches applied to -fixes, thanks.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-12-17 11:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-07 10:43 [PATCH 1/2] drm/i915: Clear self-refresh watermarks when disabled Chris Wilson
2012-12-07 10:43 ` [PATCH 2/2] drm/i915: Prefer CRTC 'active' rather than 'enabled' during WM computations Chris Wilson
2012-12-10 20:58 ` Jesse Barnes
2012-12-17 11:39 ` Daniel Vetter
2012-12-10 20:57 ` [PATCH 1/2] drm/i915: Clear self-refresh watermarks when disabled Jesse Barnes
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).