Re: [PATCH] [RFC] drm/i915/skl+: enable PF_ID interlace mode in SKL

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 07, 2017 at 03:19:05PM +0530, Mahesh Kumar wrote:
> In previous GEN default Interlace mode enabled is IF-ID mode, but IF-ID
> mode has many limitations in SKL. This mode doesn't support y-tiling,
> 90-270 rotation is not supported & YUV-420 planar source pixel formats
> are not supported with above mode.

I think we'll want something much more simple for stable. And that
should probably be just -EINVAL if we try to do any of those
forbidden things with an interlaced mode.

> 
> This patch make changes to use PF-ID Interlace mode in SKL+ platforms.
> PF-ID mode require one scaler to be attached in pipe-scaling mode.
>   During WM calculation adjusted pixel rate need to be doubled.
>   During max_supported_pixel_format calculation vertical downscale is 2.0.
> 
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@xxxxxxxxx>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90238
> ---
>  drivers/gpu/drm/i915/intel_atomic.c  | 13 ++++++++++
>  drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_drv.h     | 10 ++++++++
>  drivers/gpu/drm/i915/intel_panel.c   |  9 +++++--
>  drivers/gpu/drm/i915/intel_pm.c      | 17 ++++++++++++-
>  5 files changed, 87 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index d791b3ef89b5..06ed2fc449d7 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -248,6 +248,13 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
>  		return -EINVAL;
>  	}
>  
> +	if (num_scalers_need > 1 &&
> +			scaler_state->scaler_users & (1 << SKL_CRTC_INDEX) &&
> +			scaler_state->scaler_users & (1 << SKL_PF_ID_INDEX)) {
> +		DRM_ERROR("Can't attach scaler to CRTC in Interlace Mode\n");
> +		return -EINVAL;
> +	}

I thought the whole point of PF-ID here was that we can then do pipe
scaling (and other things). So this check looks wrong to me.

And I'm thinking we shouldn't be adding yet another scaler user for
this, and instead it should just be part of the normal pfit setup.

> +
>  	/* walkthrough scaler_users bits and start assigning scalers */
>  	for (i = 0; i < sizeof(scaler_state->scaler_users) * 8; i++) {
>  		int *scaler_id;
> @@ -264,6 +271,12 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
>  
>  			/* panel fitter case: assign as a crtc scaler */
>  			scaler_id = &scaler_state->scaler_id;
> +		} else if (i == SKL_PF_ID_INDEX) {
> +			name = "PF-ID INTERLACE MODE";
> +			idx = intel_crtc->base.base.id;
> +
> +			/* PF-ID interlaced mode case: needs a pipe scaler */
> +			scaler_id = &scaler_state->scaler_id;
>  		} else {
>  			name = "PLANE";
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 85ac32549e85..15c79b46d645 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4626,6 +4626,11 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
>  	 */
>  	need_scaling = src_w != dst_w || src_h != dst_h;
>  
> +	if ((crtc_state->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
> +		&& scaler_user == SKL_PF_ID_INDEX)
> +		/* PF-ID Interlace mode needs a scaler */
> +		need_scaling = true;
> +
>  	/*
>  	 * if plane is being disabled or scaler is no more required or force detach
>  	 *  - free scaler binded to this plane/crtc
> @@ -4635,16 +4640,24 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
>  	 * scaler can be assigned to other user. Actual register
>  	 * update to free the scaler is done in plane/panel-fit programming.
>  	 * For this purpose crtc/plane_state->scaler_id isn't reset here.
> +	 * Now Interlace mode is a new user of pipe-scaler, so free the scaler-id
> +	 * only if call is for respective user. If no user for scaler free it.
>  	 */
>  	if (force_detach || !need_scaling) {
> -		if (*scaler_id >= 0) {
> +		if (*scaler_id >= 0 &&
> +		    scaler_state->scaler_users & (1 << scaler_user)) {
>  			scaler_state->scaler_users &= ~(1 << scaler_user);
>  			scaler_state->scalers[*scaler_id].in_use = 0;
>  
>  			DRM_DEBUG_KMS("scaler_user index %u.%u: "
>  				"Staged freeing scaler id %d scaler_users = 0x%x\n",
> -				intel_crtc->pipe, scaler_user, *scaler_id,
> -				scaler_state->scaler_users);
> +				intel_crtc->pipe, scaler_user,
> +				*scaler_id, scaler_state->scaler_users);
> +
> +			*scaler_id = -1;
> +		}
> +		if (*scaler_id >= 0 && !scaler_state->scaler_users) {
> +			scaler_state->scalers[*scaler_id].in_use = 0;
>  			*scaler_id = -1;
>  		}
>  		return 0;
> @@ -4691,6 +4704,16 @@ int skl_update_scaler_crtc(struct intel_crtc_state *state)
>  		adjusted_mode->crtc_hdisplay, adjusted_mode->crtc_vdisplay);
>  }
>  
> +static int skl_update_scaler_crtc_pf_id_output(struct intel_crtc_state *state)
> +{
> +	const struct drm_display_mode *mode = &state->base.adjusted_mode;
> +
> +	return skl_update_scaler(state, !state->base.active,
> +		SKL_PF_ID_INDEX, &state->scaler_state.scaler_id,
> +		state->pipe_src_w, state->pipe_src_h,
> +		mode->crtc_hdisplay, mode->crtc_vdisplay);
> +}
> +
>  /**
>   * skl_update_scaler_plane - Stages update to scaler state for a given plane.
>   *
> @@ -6258,6 +6281,15 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  		}
>  	}
>  
> +	if (INTEL_GEN(dev_priv) >= 9) {
> +		if (skl_update_scaler_crtc_pf_id_output(pipe_config)) {
> +			DRM_ERROR("Unable to set scaler for PF-ID mode\n");
> +			return -EINVAL;
> +		}
> +		intel_pch_panel_fitting(crtc, pipe_config,
> +					DRM_MODE_SCALE_FULLSCREEN);
> +	}
> +
>  	if (adjusted_mode->crtc_clock > clock_limit) {
>  		DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high (max: %d kHz, double wide: %s)\n",
>  			      adjusted_mode->crtc_clock, clock_limit,
> @@ -8045,9 +8077,12 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc)
>  	if (IS_HASWELL(dev_priv) && intel_crtc->config->dither)
>  		val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
>  
> -	if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
> -		val |= PIPECONF_INTERLACED_ILK;
> -	else
> +	if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
> +		if (INTEL_GEN(dev_priv) >= 9)
> +			val |= PIPECONF_PFIT_PF_INTERLACED_ILK;
> +		else
> +			val |= PIPECONF_INTERLACED_ILK;
> +	} else
>  		val |= PIPECONF_PROGRESSIVE;
>  
>  	I915_WRITE(PIPECONF(cpu_transcoder), val);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 83dd40905821..d4546f6498b9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -483,6 +483,7 @@ struct intel_crtc_scaler_state {
>  	 * avilability.
>  	 */
>  #define SKL_CRTC_INDEX 31
> +#define SKL_PF_ID_INDEX 29
>  	unsigned scaler_users;
>  
>  	/* scaler used by crtc for panel fitting purpose */
> @@ -1206,6 +1207,15 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
>  	return container_of(intel_hdmi, struct intel_digital_port, hdmi);
>  }
>  
> +static inline bool
> +is_skl_interlace_mode(struct drm_i915_private *dev_priv,
> +		      const struct drm_display_mode *mode)
> +{
> +	if (INTEL_GEN(dev_priv) < 9)
> +		return false;
> +	return mode->flags & DRM_MODE_FLAG_INTERLACE;
> +}
> +
>  /* intel_fifo_underrun.c */
>  bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
>  					   enum pipe pipe, bool enable);
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 4114cb3f14e7..fae7fe7802f8 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -108,9 +108,14 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
>  	const struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
>  	int x = 0, y = 0, width = 0, height = 0;
>  
> -	/* Native modes don't need fitting */
> +	/*
> +	 * Native modes don't need fitting
> +	 * PF-ID Interlace mode in SKL+ require a pipe scaler
> +	 */
>  	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
> -	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h)
> +	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h &&
> +	    !(is_skl_interlace_mode(to_i915(intel_crtc->base.dev),
> +				    adjusted_mode)))
>  		goto done;
>  
>  	switch (fitting_mode) {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index aa9d8cef7ce0..dfb21f00fbed 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3873,15 +3873,18 @@ static uint_fixed_16_16_t
>  skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state)
>  {
>  	uint_fixed_16_16_t pipe_downscale = u32_to_fixed_16_16(1);
> +	const struct drm_display_mode *mode;
>  
>  	if (!crtc_state->base.enable)
>  		return pipe_downscale;
>  
> +	mode = &crtc_state->base.adjusted_mode;
>  	if (crtc_state->pch_pfit.enabled) {
>  		uint32_t src_w, src_h, dst_w, dst_h;
>  		uint32_t pfit_size = crtc_state->pch_pfit.size;
>  		uint_fixed_16_16_t fp_w_ratio, fp_h_ratio;
>  		uint_fixed_16_16_t downscale_h, downscale_w;
> +		uint_fixed_16_16_t min_h_downscale;
>  
>  		src_w = crtc_state->pipe_src_w;
>  		src_h = crtc_state->pipe_src_h;
> @@ -3891,10 +3894,19 @@ skl_pipe_downscale_amount(const struct intel_crtc_state *crtc_state)
>  		if (!dst_w || !dst_h)
>  			return pipe_downscale;
>  
> +		/*
> +		 * Interlace mode require 1 scaler so no other scaler can be
> +		 * attached to pipe. PF-ID mode is equivalent to 2.0 vertical
> +		 * downscale.
> +		 */
> +		if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> +			min_h_downscale = u32_to_fixed_16_16(2);
> +		else
> +			min_h_downscale = u32_to_fixed_16_16(1);
>  		fp_w_ratio = fixed_16_16_div(src_w, dst_w);
>  		fp_h_ratio = fixed_16_16_div(src_h, dst_h);
>  		downscale_w = max_fixed_16_16(fp_w_ratio, u32_to_fixed_16_16(1));
> -		downscale_h = max_fixed_16_16(fp_h_ratio, u32_to_fixed_16_16(1));
> +		downscale_h = max_fixed_16_16(fp_h_ratio, min_h_downscale);
>  
>  		pipe_downscale = mul_fixed16(downscale_w, downscale_h);
>  	}
> @@ -4418,6 +4430,9 @@ skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *cstate,
>  	 * with additional adjustments for plane-specific scaling.
>  	 */
>  	adjusted_pixel_rate = cstate->pixel_rate;
> +	if (cstate->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE)
> +		adjusted_pixel_rate *= 2;
> +
>  	downscale_amount = skl_plane_downscale_amount(cstate, pstate);
>  
>  	return mul_round_up_u32_fixed16(adjusted_pixel_rate,
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux