Re: [PATCH 17/42] drm/i915: Remove use of crtc->config from intel_pm.c

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, May 11, 2015 at 04:24:53PM +0200, Maarten Lankhorst wrote:

A comment here stating that the wm code doesn't check/precompute anything
yet, hence we always need to look at the current config would be good.

/me on a crusade against terse commit messages

Cheers, Daniel

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 96 ++++++++++++++++++++---------------------
>  1 file changed, 46 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a70b2d1fc844..869b587f463d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -678,7 +678,7 @@ static void pineview_update_wm(struct drm_crtc *unused_crtc)
>  		int pixel_size = crtc->primary->state->fb->bits_per_pixel / 8;
>  		int clock;
>  
> -		adjusted_mode = &to_intel_crtc(crtc)->config->base.adjusted_mode;
> +		adjusted_mode = &crtc->state->adjusted_mode;
>  		clock = adjusted_mode->crtc_clock;
>  
>  		/* Display SR */
> @@ -747,10 +747,10 @@ static bool g4x_compute_wm0(struct drm_device *dev,
>  		return false;
>  	}
>  
> -	adjusted_mode = &to_intel_crtc(crtc)->config->base.adjusted_mode;
> +	adjusted_mode = &crtc->state->adjusted_mode;
>  	clock = adjusted_mode->crtc_clock;
>  	htotal = adjusted_mode->crtc_htotal;
> -	hdisplay = to_intel_crtc(crtc)->config->pipe_src_w;
> +	hdisplay = to_intel_crtc_state(crtc->state)->pipe_src_w;
>  	pixel_size = crtc->primary->state->fb->bits_per_pixel / 8;
>  
>  	/* Use the small buffer method to calculate plane watermark */
> @@ -834,10 +834,10 @@ static bool g4x_compute_srwm(struct drm_device *dev,
>  	}
>  
>  	crtc = intel_get_crtc_for_plane(dev, plane);
> -	adjusted_mode = &to_intel_crtc(crtc)->config->base.adjusted_mode;
> +	adjusted_mode = &crtc->state->adjusted_mode;
>  	clock = adjusted_mode->crtc_clock;
>  	htotal = adjusted_mode->crtc_htotal;
> -	hdisplay = to_intel_crtc(crtc)->config->pipe_src_w;
> +	hdisplay = to_intel_crtc_state(crtc->state)->pipe_src_w;
>  	pixel_size = crtc->primary->state->fb->bits_per_pixel / 8;
>  
>  	line_time_us = max(htotal * 1000 / clock, 1);
> @@ -934,16 +934,11 @@ static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc,
>  					 struct drm_plane *plane)
>  {
>  	struct drm_device *dev = crtc->dev;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int entries, prec_mult, drain_latency, pixel_size;
> -	int clock = intel_crtc->config->base.adjusted_mode.crtc_clock;
> +	int clock = crtc->state->adjusted_mode.crtc_clock;
>  	const int high_precision = IS_CHERRYVIEW(dev) ? 16 : 64;
>  
> -	/*
> -	 * FIXME the plane might have an fb
> -	 * but be invisible (eg. due to clipping)
> -	 */
> -	if (!intel_crtc->active || !plane->state->fb)
> +	if (!crtc->state->active || !to_intel_plane_state(plane->state)->visible)
>  		return 0;
>  
>  	if (WARN(clock == 0, "Pixel clock is zero!\n"))
> @@ -981,11 +976,11 @@ static int vlv_compute_wm(struct intel_crtc *crtc,
>  	 * FIXME the plane might have an fb
>  	 * but be invisible (eg. due to clipping)
>  	 */
> -	if (!crtc->active || !plane->base.state->fb)
> +	if (!crtc->base.state->active || !plane->base.state->fb)
>  		return 0;
>  
>  	pixel_size = drm_format_plane_cpp(plane->base.state->fb->pixel_format, 0);
> -	clock = crtc->config->base.adjusted_mode.crtc_clock;
> +	clock = crtc->base.state->adjusted_mode.crtc_clock;
>  
>  	entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
>  
> @@ -1217,10 +1212,10 @@ static void i965_update_wm(struct drm_crtc *unused_crtc)
>  		/* self-refresh has much higher latency */
>  		static const int sr_latency_ns = 12000;
>  		const struct drm_display_mode *adjusted_mode =
> -			&to_intel_crtc(crtc)->config->base.adjusted_mode;
> +			&crtc->state->adjusted_mode;
>  		int clock = adjusted_mode->crtc_clock;
>  		int htotal = adjusted_mode->crtc_htotal;
> -		int hdisplay = to_intel_crtc(crtc)->config->pipe_src_w;
> +		int hdisplay = to_intel_crtc_state(crtc->state)->pipe_src_w;
>  		int pixel_size = crtc->primary->state->fb->bits_per_pixel / 8;
>  		unsigned long line_time_us;
>  		int entries;
> @@ -1304,7 +1299,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
>  		if (IS_GEN2(dev))
>  			cpp = 4;
>  
> -		adjusted_mode = &to_intel_crtc(crtc)->config->base.adjusted_mode;
> +		adjusted_mode = &crtc->state->adjusted_mode;
>  		planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
>  					       wm_info, fifo_size, cpp,
>  					       pessimal_latency_ns);
> @@ -1326,7 +1321,7 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
>  		if (IS_GEN2(dev))
>  			cpp = 4;
>  
> -		adjusted_mode = &to_intel_crtc(crtc)->config->base.adjusted_mode;
> +		adjusted_mode = &crtc->state->adjusted_mode;
>  		planeb_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
>  					       wm_info, fifo_size, cpp,
>  					       pessimal_latency_ns);
> @@ -1365,10 +1360,10 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
>  		/* self-refresh has much higher latency */
>  		static const int sr_latency_ns = 6000;
>  		const struct drm_display_mode *adjusted_mode =
> -			&to_intel_crtc(enabled)->config->base.adjusted_mode;
> +			&enabled->state->adjusted_mode;
>  		int clock = adjusted_mode->crtc_clock;
>  		int htotal = adjusted_mode->crtc_htotal;
> -		int hdisplay = to_intel_crtc(enabled)->config->pipe_src_w;
> +		int hdisplay = to_intel_crtc_state(enabled->state)->pipe_src_w;
>  		int pixel_size = enabled->primary->state->fb->bits_per_pixel / 8;
>  		unsigned long line_time_us;
>  		int entries;
> @@ -1421,7 +1416,7 @@ static void i845_update_wm(struct drm_crtc *unused_crtc)
>  	if (crtc == NULL)
>  		return;
>  
> -	adjusted_mode = &to_intel_crtc(crtc)->config->base.adjusted_mode;
> +	adjusted_mode = &crtc->state->adjusted_mode;
>  	planea_wm = intel_calculate_wm(adjusted_mode->crtc_clock,
>  				       &i845_wm_info,
>  				       dev_priv->display.get_fifo_size(dev, 0),
> @@ -1437,20 +1432,20 @@ static void i845_update_wm(struct drm_crtc *unused_crtc)
>  static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
>  				    struct drm_crtc *crtc)
>  {
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	uint32_t pixel_rate;
> -
> -	pixel_rate = intel_crtc->config->base.adjusted_mode.crtc_clock;
> +	uint32_t pixel_rate =
> +		crtc->state->adjusted_mode.crtc_clock;
> +	struct intel_crtc_state *pipe_config =
> +		to_intel_crtc_state(crtc->state);
>  
>  	/* We only use IF-ID interlacing. If we ever use PF-ID we'll need to
>  	 * adjust the pixel_rate here. */
>  
> -	if (intel_crtc->config->pch_pfit.enabled) {
> +	if (pipe_config->pch_pfit.enabled) {
>  		uint64_t pipe_w, pipe_h, pfit_w, pfit_h;
> -		uint32_t pfit_size = intel_crtc->config->pch_pfit.size;
> +		uint32_t pfit_size = pipe_config->pch_pfit.size;
>  
> -		pipe_w = intel_crtc->config->pipe_src_w;
> -		pipe_h = intel_crtc->config->pipe_src_h;
> +		pipe_w = pipe_config->pipe_src_w;
> +		pipe_h = pipe_config->pipe_src_h;
>  		pfit_w = (pfit_size >> 16) & 0xFFFF;
>  		pfit_h = pfit_size & 0xFFFF;
>  		if (pipe_w < pfit_w)
> @@ -1802,11 +1797,10 @@ static uint32_t
>  hsw_compute_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct drm_display_mode *mode = &intel_crtc->config->base.adjusted_mode;
> +	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
>  	u32 linetime, ips_linetime;
>  
> -	if (!intel_crtc->active)
> +	if (!crtc->state->active)
>  		return 0;
>  
>  	/* The WM are computed with base on how long it takes to fill a single
> @@ -2061,11 +2055,11 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
>  	enum pipe pipe = intel_crtc->pipe;
>  	struct drm_plane *plane;
>  
> -	if (!intel_crtc->active)
> +	if (!crtc->state->active)
>  		return;
>  
>  	p->active = true;
> -	p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
> +	p->pipe_htotal = crtc->state->adjusted_mode.crtc_htotal;
>  	p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
>  
>  	if (crtc->primary->state->fb) {
> @@ -2084,8 +2078,8 @@ static void ilk_compute_wm_parameters(struct drm_crtc *crtc,
>  		p->cur.enabled = false;
>  		p->cur.bytes_per_pixel = 0;
>  	}
> -	p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
> -	p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
> +	p->pri.horiz_pixels = to_intel_crtc_state(crtc->state)->pipe_src_w;
> +	p->cur.horiz_pixels = crtc->cursor->state->crtc_w;
>  
>  	drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
>  		struct intel_plane *intel_plane = to_intel_plane(plane);
> @@ -2589,7 +2583,7 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
>  
>  	nth_active_pipe = 0;
>  	for_each_crtc(dev, crtc) {
> -		if (!to_intel_crtc(crtc)->active)
> +		if (!crtc->state->active)
>  			continue;
>  
>  		if (crtc == for_crtc)
> @@ -2862,7 +2856,7 @@ static void skl_compute_wm_global_parameters(struct drm_device *dev,
>  	struct drm_plane *plane;
>  
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> -		config->num_pipes_active += to_intel_crtc(crtc)->active;
> +		config->num_pipes_active += crtc->state->active;
>  
>  	/* FIXME: I don't think we need those two global parameters on SKL */
>  	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> @@ -2881,12 +2875,14 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
>  	enum pipe pipe = intel_crtc->pipe;
>  	struct drm_plane *plane;
>  	struct drm_framebuffer *fb;
> +	struct intel_crtc_state *pipe_config;
>  	int i = 1; /* Index for sprite planes start */
>  
> -	p->active = intel_crtc->active;
> +	pipe_config = to_intel_crtc_state(crtc->state);
> +	p->active = crtc->state->active;
>  	if (p->active) {
> -		p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
> -		p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config);
> +		p->pipe_htotal = pipe_config->base.adjusted_mode.crtc_htotal;
> +		p->pixel_rate = skl_pipe_pixel_rate(pipe_config);
>  
>  		fb = crtc->primary->state->fb;
>  		/* For planar: Bpp is for uv plane, y_Bpp is for y plane */
> @@ -2903,8 +2899,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
>  			p->plane[0].y_bytes_per_pixel = 0;
>  			p->plane[0].tiling = DRM_FORMAT_MOD_NONE;
>  		}
> -		p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w;
> -		p->plane[0].vert_pixels = intel_crtc->config->pipe_src_h;
> +		p->plane[0].horiz_pixels = pipe_config->pipe_src_w;
> +		p->plane[0].vert_pixels = pipe_config->pipe_src_h;
>  		p->plane[0].rotation = crtc->primary->state->rotation;
>  
>  		fb = crtc->cursor->state->fb;
> @@ -3042,7 +3038,7 @@ static void skl_compute_wm_level(const struct drm_i915_private *dev_priv,
>  static uint32_t
>  skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
>  {
> -	if (!to_intel_crtc(crtc)->active)
> +	if (!crtc->state->active)
>  		return 0;
>  
>  	return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
> @@ -3266,7 +3262,7 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
>  	 * re-allocate the freed space without this pipe fetching from it.
>  	 */
>  	for_each_intel_crtc(dev, crtc) {
> -		if (!crtc->active)
> +		if (!crtc->base.state->active)
>  			continue;
>  
>  		pipe = crtc->pipe;
> @@ -3289,7 +3285,7 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
>  	 * space is not used anymore.
>  	 */
>  	for_each_intel_crtc(dev, crtc) {
> -		if (!crtc->active)
> +		if (!crtc->base.state->active)
>  			continue;
>  
>  		pipe = crtc->pipe;
> @@ -3312,7 +3308,7 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
>  	 * will just get more DDB space with the correct WM values.
>  	 */
>  	for_each_intel_crtc(dev, crtc) {
> -		if (!crtc->active)
> +		if (!crtc->base.state->active)
>  			continue;
>  
>  		pipe = crtc->pipe;
> @@ -3378,7 +3374,7 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
>  		if (this_crtc->pipe == intel_crtc->pipe)
>  			continue;
>  
> -		if (!intel_crtc->active)
> +		if (!intel_crtc->base.state->active)
>  			continue;
>  
>  		wm_changed = skl_update_pipe_wm(&intel_crtc->base,
> @@ -3603,7 +3599,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
>  		hw->plane_trans[pipe][i] = I915_READ(PLANE_WM_TRANS(pipe, i));
>  	hw->cursor_trans[pipe] = I915_READ(CUR_WM_TRANS(pipe));
>  
> -	if (!intel_crtc->active)
> +	if (!crtc->state->active)
>  		return;
>  
>  	hw->dirty[pipe] = true;
> @@ -3658,7 +3654,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
>  	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>  		hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
>  
> -	active->pipe_enabled = intel_crtc->active;
> +	active->pipe_enabled = crtc->state->active;
>  
>  	if (active->pipe_enabled) {
>  		u32 tmp = hw->wm_pipe[pipe];
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx