Re: [RFC 4/6] drm/i915/vlv: Change to use intel_crtc_state instead of base CRTC object

[Matt Roper <matthew.d.roper@intel.com>]

On Wed, Jun 08, 2016 at 05:22:42PM +0200, Chi Ding wrote:
> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> This commit changs some functions to operate on intel_crtc_state rather
> than the base CRTC objects in order to transit to atomic

I'd add a little bit more justification to this message.

The reason we want to do this is to allow future patches to move the
computation steps into the atomic 'check' phase where they'll be
operating on in-flight CRTC states rather than already-committed states.

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Chi Ding <chix.ding@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 33f52ae..3fc80f9 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -983,9 +983,10 @@ static uint16_t vlv_compute_wm_level(struct intel_plane *plane,
>  	return min_t(int, wm, USHRT_MAX);
>  }
>  
> -static void vlv_compute_fifo(struct intel_crtc *crtc,
> +static void vlv_compute_fifo(struct intel_crtc_state *cstate,
>  				struct vlv_wm_state *wm_state)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	struct intel_plane *plane;
>  	unsigned int total_rate = 0;
> @@ -1088,8 +1089,9 @@ static void vlv_invert_wms(struct intel_crtc *crtc,
>  	}
>  }
>  
> -static int vlv_compute_wm(struct intel_crtc *crtc)
> +static int vlv_compute_wm(struct intel_crtc_state *cstate)
>  {
> +	struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	struct vlv_wm_state *wm_state = &crtc->wm_state;
>  	struct intel_plane *plane;
> @@ -1103,7 +1105,7 @@ static int vlv_compute_wm(struct intel_crtc *crtc)
>  
>  	wm_state->num_active_planes = 0;
>  
> -	vlv_compute_fifo(crtc, wm_state);
> +	vlv_compute_fifo(cstate, wm_state);
>  
>  	if (wm_state->num_active_planes != 1)
>  		wm_state->cxsr = false;
> @@ -1131,7 +1133,7 @@ static int vlv_compute_wm(struct intel_crtc *crtc)
>  				DRM_DEBUG_KMS("Requested display configuration "
>  				"exceeds system watermark limitations\n");
>  				DRM_DEBUG_KMS("Plane %d.%d: blocks required = %u/%u\n",
> -				      crtc->pipe,
> +				      to_intel_crtc(cstate->base.crtc)->pipe,

This change is unnecessary, right?  'crtc' is still initialized to the
same thing at the top of the function.


Matt

>  				      drm_plane_index(&plane->base), wm, max_wm);
>  				return -EINVAL;
>  			}
> @@ -1332,7 +1334,7 @@ static void vlv_update_wm(struct drm_crtc *crtc)
>  	enum pipe pipe = intel_crtc->pipe;
>  	struct vlv_wm_values wm = {};
>  
> -	vlv_compute_wm(intel_crtc);
> +	vlv_compute_wm(intel_crtc->config);
>  	vlv_merge_wm(dev, &wm);
>  
>  	if (memcmp(&dev_priv->wm.vlv, &wm, sizeof(wm)) == 0) {
> -- 
> 1.8.0.1
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx