Intel-GFX Archive on lore.kernel.org
 help / color / Atom feed
From: Jesse Barnes <jbarnes@virtuousgeek.org>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/i915: Prefer CRTC 'active' rather than 'enabled' during WM computations
Date: Mon, 10 Dec 2012 12:58:40 -0800
Message-ID: <20121210125840.7740b32e@jbarnes-desktop> (raw)
In-Reply-To: <1354877005-31477-2-git-send-email-chris@chris-wilson.co.uk>

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

  reply index

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121210125840.7740b32e@jbarnes-desktop \
    --to=jbarnes@virtuousgeek.org \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Intel-GFX Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/intel-gfx/0 intel-gfx/git/0.git
	git clone --mirror https://lore.kernel.org/intel-gfx/1 intel-gfx/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 intel-gfx intel-gfx/ https://lore.kernel.org/intel-gfx \
		intel-gfx@lists.freedesktop.org
	public-inbox-index intel-gfx

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.freedesktop.lists.intel-gfx


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git