diff mbox

[2/2] drm/i915: Prefer CRTC 'active' rather than 'enabled' during WM computations

Message ID 1354877005-31477-2-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Dec. 7, 2012, 10:43 a.m. UTC
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(-)

Comments

Jesse Barnes Dec. 10, 2012, 8:58 p.m. UTC | #1
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>
Daniel Vetter Dec. 17, 2012, 11:39 a.m. UTC | #2
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
diff mbox

Patch

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;
 	}