Re: [PATCH 3/4] drm/i915: add modeset_global_pipes callback for global config adjustments

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Nov 01, 2013 at 08:41:26AM -0700, Jesse Barnes wrote:
> In some cases we may need to turn off more pipes than just the ones
> affected by a connector or encoder change, probably due to modifying
> some global resource that requires all pipes to shut down.  Add a hook
> for this purpose to allow platform code to adjust the prepare_pipes
> mask, which allows otherwise active pipes to get shut down for the
> duration of the mode set, then restored at the end.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  7 +++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5edf9bb..7530cd4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -380,6 +380,18 @@ struct drm_i915_display_funcs {
>  				 struct drm_crtc *crtc,
>  				 uint32_t sprite_width, int pixel_size,
>  				 bool enable, bool scaled);
> +	/**
> +	 * modeset_global_pipes - figure out which pipes need to be disabled
> +	 * @dev: drm device
> +	 * @prepare_pipes: bitmask of pipes to disable
> +	 *
> +	 * In some cases, adjusting a global resource may require shutting
> +	 * down seemingly unrelated pipes on a mode set.  This function
> +	 * should determine that and adjust the prepare_pipes bitmask
> +	 * as needed.
> +	 */
> +	void (*modeset_global_pipes)(struct drm_device *dev,
> +				     unsigned *prepare_pipes);
>  	void (*modeset_global_resources)(struct drm_device *dev);
>  	/* Returns the active state of the crtc, and if the crtc is active,
>  	 * fills out the pipe-config with the hw state. */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 606a594..faa7548 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9298,6 +9298,13 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>  	intel_modeset_affected_pipes(crtc, &modeset_pipes,
>  				     &prepare_pipes, &disable_pipes);
>  
> +	/*
> +	 * See if the config requires any additional preparation, e.g.
> +	 * to adjust global state with pipes off.
> +	 */
> +	if (dev_priv->display.modeset_global_pipes)
> +		dev_priv->display.modeset_global_pipes(dev, &prepare_pipes);

tbh I'd have just added a small callback here - as long as all the global
modeset config stuff is still in flux I wouldn't bother with fancy-looking
callbacks which we need to change later on anyway. And avoiding the vfunc
helps with following the code a bit ...
-Daniel

> +
>  	*saved_hwmode = crtc->hwmode;
>  	*saved_mode = crtc->mode;
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch