All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jesse Barnes <jbarnes@virtuousgeek.org>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 51/58] drm/i915: implement new set_mode code flow
Date: Wed, 5 Sep 2012 11:14:00 -0700	[thread overview]
Message-ID: <20120905111400.77780241@jbarnes-desktop> (raw)
In-Reply-To: <1345403595-9678-52-git-send-email-daniel.vetter@ffwll.ch>

On Sun, 19 Aug 2012 21:13:08 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> ... using the pipe masks from the previous patch.
> 
> Well, not quite:
> - We still need to call the disable_unused_functions helper, until
>   we've moved the call to commit_output_state further down and
>   adjusted intel_crtc_disable a bit. The next patch will do that.
> - Because we don't support (yet) mode changes on more than one crtc at
>   a time, some of the modeset_pipes checks are a bit hackish - but
>   that only needs fixing once we incorporate global modeset support.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 99 +++++++++++++++++++++++-------------
>  1 file changed, 63 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4334400..3d99522 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6791,6 +6791,12 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
>  	*prepare_pipes &= ~(*disable_pipes);
>  }
>  
> +#define for_each_intel_crtc_masked(dev, mask, intel_crtc) \
> +	list_for_each_entry((intel_crtc), \
> +			    &(dev)->mode_config.crtc_list, \
> +			    base.head) \
> +		if (mask & (1 <<(intel_crtc)->pipe)) \
> +
>  bool intel_set_mode(struct drm_crtc *crtc,
>  		    struct drm_display_mode *mode,
>  		    int x, int y, struct drm_framebuffer *fb)
> @@ -6800,73 +6806,92 @@ bool intel_set_mode(struct drm_crtc *crtc,
>  	struct drm_display_mode *adjusted_mode, saved_mode, saved_hwmode;
>  	struct drm_encoder_helper_funcs *encoder_funcs;
>  	struct drm_encoder *encoder;
> -	unsigned disable_pipe, prepare_pipes, modeset_pipes;
> +	struct intel_crtc *intel_crtc;
> +	unsigned disable_pipes, prepare_pipes, modeset_pipes;
>  	bool ret = true;
>  
>  	intel_modeset_affected_pipes(crtc, &modeset_pipes,
> -				     &prepare_pipes, &disable_pipe);
> +				     &prepare_pipes, &disable_pipes);
> +
> +	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
> +		      modeset_pipes, prepare_pipes, disable_pipes);
>  
>  	intel_modeset_commit_output_state(dev);
>  
>  	crtc->enabled = drm_helper_crtc_in_use(crtc);
> -	if (!crtc->enabled) {
> -		drm_helper_disable_unused_functions(dev);
> -		return true;
> -	}
>  
> +	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
> +		drm_helper_disable_unused_functions(dev);
>  
>  	saved_hwmode = crtc->hwmode;
>  	saved_mode = crtc->mode;
>  
> -	adjusted_mode = intel_modeset_adjusted_mode(crtc, mode);
> -	if (IS_ERR(adjusted_mode)) {
> -		return false;
> -	}
> +	/* Hack: Because we don't (yet) support global modeset on multiple
> +	 * crtcs, we don't keep track of the new mode for more than one crtc.
> +	 * Hence simply check whether any bit is set in modeset_pipes in all the
> +	 * pieces of code that are not yet converted to deal with mutliple crtcs
> +	 * changing their mode at the same time. */
> +	adjusted_mode = NULL;
> +	if (modeset_pipes) {
> +		adjusted_mode = intel_modeset_adjusted_mode(crtc, mode);
> +		if (IS_ERR(adjusted_mode)) {
> +			return false;
> +		}
>  
> -	intel_crtc_prepare_encoders(dev);
> +		intel_crtc_prepare_encoders(dev);
> +	}
>  
> -	dev_priv->display.crtc_disable(crtc);
> +	for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc)
> +		dev_priv->display.crtc_disable(&intel_crtc->base);
>  
> -	crtc->mode = *mode;
> +	if (modeset_pipes) {
> +		crtc->mode = *mode;
> +		crtc->x = x;
> +		crtc->y = y;
> +	}
>  
>  	/* Set up the DPLL and any encoders state that needs to adjust or depend
>  	 * on the DPLL.
>  	 */
> -	ret = !intel_crtc_mode_set(crtc, mode, adjusted_mode, x, y, fb);
> -	if (!ret)
> -	    goto done;
> +	for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
> +		ret = !intel_crtc_mode_set(&intel_crtc->base,
> +					   mode, adjusted_mode,
> +					   x, y, fb);
> +		if (!ret)
> +		    goto done;
>  
> -	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
> +		list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
>  
> -		if (encoder->crtc != crtc)
> -			continue;
> +			if (encoder->crtc != &intel_crtc->base)
> +				continue;
>  
> -		DRM_DEBUG_KMS("[ENCODER:%d:%s] set [MODE:%d:%s]\n",
> -			encoder->base.id, drm_get_encoder_name(encoder),
> -			mode->base.id, mode->name);
> -		encoder_funcs = encoder->helper_private;
> -		encoder_funcs->mode_set(encoder, mode, adjusted_mode);
> +			DRM_DEBUG_KMS("[ENCODER:%d:%s] set [MODE:%d:%s]\n",
> +				encoder->base.id, drm_get_encoder_name(encoder),
> +				mode->base.id, mode->name);
> +			encoder_funcs = encoder->helper_private;
> +			encoder_funcs->mode_set(encoder, mode, adjusted_mode);
> +		}
>  	}
>  
> -	crtc->x = x;
> -	crtc->y = y;
> -
>  	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
> -	dev_priv->display.crtc_enable(crtc);
> +	for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc)
> +		dev_priv->display.crtc_enable(&intel_crtc->base);
>  
> -	/* Store real post-adjustment hardware mode. */
> -	crtc->hwmode = *adjusted_mode;
> +	if (modeset_pipes) {
> +		/* Store real post-adjustment hardware mode. */
> +		crtc->hwmode = *adjusted_mode;
>  
> -	/* Calculate and store various constants which
> -	 * are later needed by vblank and swap-completion
> -	 * timestamping. They are derived from true hwmode.
> -	 */
> -	drm_calc_timestamping_constants(crtc);
> +		/* Calculate and store various constants which
> +		 * are later needed by vblank and swap-completion
> +		 * timestamping. They are derived from true hwmode.
> +		 */
> +		drm_calc_timestamping_constants(crtc);
> +	}
>  
>  	/* FIXME: add subpixel order */
>  done:
>  	drm_mode_destroy(dev, adjusted_mode);
> -	if (!ret) {
> +	if (!ret && crtc->enabled) {
>  		crtc->hwmode = saved_hwmode;
>  		crtc->mode = saved_mode;
>  	}
> @@ -6874,6 +6899,8 @@ done:
>  	return ret;
>  }
>  
> +#undef for_each_intel_crtc_masked
> +
>  static void intel_set_config_free(struct intel_set_config *config)
>  {
>  	if (config) {

An ugly intermediate step... also did you check whether moving the
crtc->x/y assignment up is safe?  We're passing it around, but some
places might check for crtc->x/y looking for old values (or did that
already change in the previous patch... too many patches).

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

  reply	other threads:[~2012-09-05 18:14 UTC|newest]

Thread overview: 173+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-19 19:12 [PATCH 00/58] modeset-rework, the basic conversion Daniel Vetter
2012-08-19 19:12 ` [PATCH 01/58] drm/i915: add crtc->enable/disable vfuncs insted of dpms Daniel Vetter
2012-08-29 17:51   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 02/58] drm/i915: rip out crtc prepare/commit indirection Daniel Vetter
2012-08-29 17:52   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 03/58] drm/i915: add direct encoder disable/enable infrastructure Daniel Vetter
2012-08-29 18:01   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 04/58] drm/i915/hdmi: convert to encoder->disable/enable Daniel Vetter
2012-09-04 19:24   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 05/58] drm/i915/tv: convert to encoder enable/disable Daniel Vetter
2012-09-04 19:25   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 06/58] drm/i915/lvds: convert to encoder disable/enable Daniel Vetter
2012-09-04 19:26   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 07/58] drm/i915/dp: " Daniel Vetter
2012-09-04 19:33   ` Jesse Barnes
2012-09-04 19:42     ` Daniel Vetter
2012-08-19 19:12 ` [PATCH 08/58] drm/i915/crt: " Daniel Vetter
2012-09-04 19:50   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 09/58] drm/i915/sdvo: " Daniel Vetter
2012-09-04 19:52   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 10/58] drm/i915/dvo: " Daniel Vetter
2012-09-04 19:53   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 11/58] drm/i915: convert dpms functions of dvo/sdvo/crt Daniel Vetter
2012-08-29  7:12   ` [PATCH] " Daniel Vetter
2012-09-04 20:10     ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 12/58] drm/i915: rip out encoder->disable/enable checks Daniel Vetter
2012-09-04 20:11   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 13/58] drm/i915: clean up encoder_prepare/commit Daniel Vetter
2012-09-04 20:12   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 14/58] drm/i915: copy&paste drm_crtc_helper_set_config Daniel Vetter
2012-09-04 20:13   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 15/58] drm/i915: call set_base directly Daniel Vetter
2012-09-04 20:15   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 16/58] drm/i915: inline intel_best_encoder Daniel Vetter
2012-09-04 20:18   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 17/58] drm/i915: copy&paste drm_crtc_helper_set_mode Daniel Vetter
2012-09-04 20:19   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 18/58] drm/i915: simplify intel_crtc_prepare_encoders Daniel Vetter
2012-09-04 20:20   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 19/58] drm/i915: rip out encoder->prepare/commit Daniel Vetter
2012-09-04 20:21   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 20/58] drm/i915: call crtc functions directly Daniel Vetter
2012-09-04 20:22   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 21/58] drm/i915: WARN when trying to enabled an unused crtc Daniel Vetter
2012-09-04 20:23   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 22/58] drm/i915: Add interfaces to read out encoder/connector hw state Daniel Vetter
2012-09-04 20:25   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 23/58] drm/i915/dp: implement get_hw_state Daniel Vetter
2012-09-04 20:26   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 24/58] drm/i915/hdmi: " Daniel Vetter
2012-09-04 20:28   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 25/58] drm/i915/tv: " Daniel Vetter
2012-09-04 20:28   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 26/58] drm/i915/lvds: " Daniel Vetter
2012-09-04 20:28   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 27/58] drm/i915/crt: " Daniel Vetter
2012-09-04 20:29   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 28/58] drm/i915/sdvo: " Daniel Vetter
2012-09-04 20:31   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 29/58] drm/i915/dvo: " Daniel Vetter
2012-09-04 20:32   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 30/58] drm/i915: read out the modeset hw state at load and resume time Daniel Vetter
2012-09-05 16:14   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 31/58] drm/i915: check connector hw/sw state Daniel Vetter
2012-09-05 16:26   ` Jesse Barnes
2012-09-05 19:10     ` Daniel Vetter
2012-08-19 19:12 ` [PATCH 32/58] drm/i915: rip out intel_crtc->dpms_mode Daniel Vetter
2012-09-05 16:27   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 33/58] drm/i915: rip out intel_dp->dpms_mode Daniel Vetter
2012-09-05 16:28   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 34/58] drm/i915: ensure the force pipe A quirk is actually followed Daniel Vetter
2012-09-05 16:32   ` Jesse Barnes
2012-09-05 19:12     ` Daniel Vetter
2012-08-19 19:12 ` [PATCH 35/58] drm/i915: introduce struct intel_set_config Daniel Vetter
2012-09-05 16:34   ` Jesse Barnes
2012-09-05 19:27     ` Daniel Vetter
2012-08-19 19:12 ` [PATCH 36/58] drm/i915: extract modeset config save/restore code Daniel Vetter
2012-09-05 16:36   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 37/58] drm/i915: extract intel_set_config_compute_mode_changes Daniel Vetter
2012-09-05 16:42   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 38/58] drm/i915: extract intel_set_config_update_output_state Daniel Vetter
2012-09-05 16:44   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 39/58] drm/i915: implement crtc helper semantics relied upon by the fb helper Daniel Vetter
2012-09-05 16:45   ` Jesse Barnes
2012-09-05 19:15     ` Daniel Vetter
2012-08-19 19:12 ` [PATCH 40/58] drm/i915: don't update the fb base if there is no fb Daniel Vetter
2012-09-05 16:47   ` Jesse Barnes
2012-08-19 19:12 ` [PATCH 41/58] drm/i915: convert pointless error checks in set_config to BUGs Daniel Vetter
2012-09-05 16:50   ` Jesse Barnes
2012-09-05 19:19     ` Daniel Vetter
2012-08-19 19:12 ` [PATCH 42/58] drm/i915: don't save all the encoder/crtc state in set_config Daniel Vetter
2012-09-05 16:52   ` Jesse Barnes
2012-09-05 19:21     ` Daniel Vetter
2012-08-19 19:13 ` [PATCH 43/58] drm/i915: stage modeset output changes Daniel Vetter
2012-09-05 17:51   ` Jesse Barnes
2012-08-19 19:13 ` [PATCH 44/58] drm/i915: push crtc->fb update into pipe_set_base Daniel Vetter
2012-09-04 19:33   ` [PATCH] " Daniel Vetter
2012-09-05 17:55     ` Jesse Barnes
2012-09-05 19:59       ` Daniel Vetter
2012-09-05 17:54   ` [PATCH 44/58] " Jesse Barnes
2012-08-19 19:13 ` [PATCH 45/58] drm/i915: remove crtc disabling special case Daniel Vetter
2012-09-05 17:56   ` Jesse Barnes
2012-08-19 19:13 ` [PATCH 46/58] drm/i915: move output commit and crtc disabling into set_mode Daniel Vetter
2012-09-05 17:58   ` Jesse Barnes
2012-08-19 19:13 ` [PATCH 47/58] drm/i915: extract adjusted mode computation Daniel Vetter
2012-09-05 18:00   ` Jesse Barnes
2012-09-05 19:30     ` Daniel Vetter
2012-08-19 19:13 ` [PATCH 48/58] drm/i915: use staged outuput config in tv->mode_fixup Daniel Vetter
2012-09-05 18:02   ` Jesse Barnes
2012-09-06  7:30     ` Daniel Vetter
2012-08-19 19:13 ` [PATCH 49/58] drm/i915: use staged outuput config in lvds->mode_fixup Daniel Vetter
2012-09-05 18:02   ` Jesse Barnes
2012-08-19 19:13 ` [PATCH 50/58] drm/i915: compute masks of crtcs affected in set_mode Daniel Vetter
2012-08-29 10:34   ` [PATCH] " Daniel Vetter
2012-09-05 18:09     ` Jesse Barnes
2012-09-05 19:38       ` Daniel Vetter
2012-09-05 19:45         ` Jesse Barnes
2012-09-05 18:07   ` [PATCH 50/58] " Jesse Barnes
2012-08-19 19:13 ` [PATCH 51/58] drm/i915: implement new set_mode code flow Daniel Vetter
2012-09-05 18:14   ` Jesse Barnes [this message]
2012-09-05 19:43     ` Daniel Vetter
2012-09-05 19:49       ` Jesse Barnes
2012-08-19 19:13 ` [PATCH 52/58] drm/i915: push commit_output_state past crtc disabling Daniel Vetter
2012-09-05 18:17   ` Jesse Barnes
2012-08-19 19:13 ` [PATCH 53/58] drm/i915: s/intel_encoder_disable/intel_encoder_noop Daniel Vetter
2012-09-05 18:17   ` Jesse Barnes
2012-08-19 19:13 ` [PATCH 54/58] drm/i915: WARN if the pipe won't turn off Daniel Vetter
2012-09-05 18:18   ` Jesse Barnes
2012-08-19 19:13 ` [PATCH 55/58] drm/i915: switch the load detect code to the staged modeset config Daniel Vetter
2012-09-05 18:19   ` Jesse Barnes
2012-08-19 19:13 ` [PATCH 56/58] drm/i915: push commit_output_state past the crtc/encoder preparing Daniel Vetter
2012-08-31 18:12   ` [PATCH] " Daniel Vetter
2012-09-04 19:32     ` Daniel Vetter
2012-09-05 18:28       ` Jesse Barnes
2012-09-05 19:48         ` Daniel Vetter
2012-09-05 18:56           ` [PATCH] drm/i915: push crtc->fb update into pipe_set_base Daniel Vetter
2012-09-05 19:50           ` [PATCH] drm/i915: push commit_output_state past the crtc/encoder preparing Jesse Barnes
2012-09-06 20:46             ` Jesse Barnes
2012-08-19 19:13 ` [PATCH 57/58] drm/i915: disable all crtcs at suspend time Daniel Vetter
2012-08-29 21:13   ` [PATCH] drm/i915: no longer call drm_helper_resume_force_mode Daniel Vetter
2012-09-05 18:31     ` Jesse Barnes
2012-09-05 19:56       ` Daniel Vetter
2012-09-05 20:04         ` Jesse Barnes
2012-09-06 20:47           ` Jesse Barnes
2012-09-05 18:29   ` [PATCH 57/58] drm/i915: disable all crtcs at suspend time Jesse Barnes
2012-08-19 19:13 ` [PATCH 58/58] drm/i915: add tons of modeset state checks Daniel Vetter
2012-08-20  8:24   ` [PATCH] " Daniel Vetter
2012-08-20 12:22   ` Daniel Vetter
2012-08-31 18:12     ` [PATCH 1/2] " Daniel Vetter
2012-08-31 18:12       ` [PATCH 2/2] drm/i915: improve modeset state checking after dpms calls Daniel Vetter
2012-09-05 18:34         ` Jesse Barnes
2012-09-05 18:33       ` [PATCH 1/2] drm/i915: add tons of modeset state checks Jesse Barnes
2012-08-20 13:17 ` [PATCH 00/58] modeset-rework, the basic conversion Jani Nikula
2012-08-21  3:27 ` Ben Widawsky
2012-08-21 17:48 ` Lespiau, Damien
2012-08-21 18:11   ` Daniel Vetter
2012-08-22 10:46     ` Lespiau, Damien
2012-08-22 11:03       ` Lespiau, Damien
2012-08-22 19:13         ` Lespiau, Damien
2012-08-22 21:21           ` Daniel Vetter
2012-08-23 12:26             ` Lespiau, Damien
2012-08-23 22:39               ` Daniel Vetter
2012-08-29 12:26                 ` Lespiau, Damien
2012-08-30 10:40                   ` Rodrigo Vivi
2012-08-21 19:48 ` Chris Wilson
2012-08-27  8:04 ` Vijay Purushothaman
2012-09-03 17:50 ` Paulo Zanoni
2012-09-05 23:23 ` Jesse Barnes
2012-09-06  6:55   ` Daniel Vetter
2012-09-06  7:23     ` Daniel Vetter
2012-09-07  1:08   ` Jesse Barnes
2012-09-07  8:55     ` Daniel Vetter
2012-09-06 21:00 ` Daniel Vetter

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=20120905111400.77780241@jbarnes-desktop \
    --to=jbarnes@virtuousgeek.org \
    --cc=daniel.vetter@ffwll.ch \
    --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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.