Re: [PATCH 01/14] drm/i915: Store the pipe pixel rate in the crtc state

[Ander Conselvan De Oliveira <conselvan2@gmail.com>]

On Mon, 2016-12-19 at 19:28 +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rather than recomptuing the pipe pixel rate on demand everwhere, let's
> just stick the precomputed value into the crtc state.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_fbc.c     |  3 +--
>  drivers/gpu/drm/i915/intel_pm.c      | 14 +++++++-------
>  4 files changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0b0d7e8be630..1d979041c52c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> 

[...]

> @@ -16919,7 +16938,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
                __drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
                memset(crtc_state, 0, sizeof(*crtc_state));
                crtc_state->base.crtc = &crtc->base;
 
                crtc_state->base.active = crtc_state->base.enable =
                        dev_priv->display.get_pipe_config(crtc, crtc_state);
 
                crtc->base.enabled = crtc_state->base.enable;
                crtc->active = crtc_state->base.active;
 
                if (crtc_state->base.active) {
>  			dev_priv->active_crtcs |= 1 << crtc->pipe;
>  
>  			if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> -				pixclk = ilk_pipe_pixel_rate(crtc_state);
> +				pixclk = crtc_state->pixel_rate;

Aren't you reading 0 here, because of the memset above? As far as I can tell,
haswell_get_pipe_config() doesn't set crtc_state->pixel_rate.

>  			else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>  				pixclk = crtc_state->base.adjusted_mode.crtc_clock;
>  			else
> @@ -17031,6 +17050,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  			 */
>  			crtc->base.state->mode.private_flags = I915_MODE_FLAG_INHERITED;
>  
> +			intel_crtc_compute_pixel_rate(crtc->config);
> +
>  			drm_calc_timestamping_constants(&crtc->base, &crtc->base.hwmode);
>  			update_scanline_offset(crtc);
>  		}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f61ea43c7532..3969e786d566 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -541,6 +541,8 @@ struct intel_crtc_state {
>  	 * and get clipped at the edges. */
>  	int pipe_src_w, pipe_src_h;
>  
> +	unsigned int pixel_rate;
> +

Maybe add some comment about this parameter. This is not in kernel doc, but
having that already would probably make it easier for whoever does it in the
end.

Ander

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx