Re: [PATCH 13/16] drm/i915: Split PPS write from DSC enable

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

 



On Mon, Sep 13, 2021 at 05:44:37PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> The PPS SDP is fed into the transcoder whereas the DSC
> block is (or at least can be) per pipe. Let's split these
> into two distinct operations in an effort to untagle the
> bigjoiner mess where we have two pipes feeding a single
> transcoder.
>

Yes makes sense.

Reviewed-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>

Manasi
 
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c       |  6 ++--
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 14 +++++++--
>  drivers/gpu/drm/i915/display/intel_display.c |  5 ++-
>  drivers/gpu/drm/i915/display/intel_dp.c      |  2 +-
>  drivers/gpu/drm/i915/display/intel_vdsc.c    | 33 +++++++++-----------
>  drivers/gpu/drm/i915/display/intel_vdsc.h    | 10 +++---
>  6 files changed, 39 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 060bc8fb0d30..070ad144ef83 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1246,7 +1246,9 @@ static void gen11_dsi_pre_enable(struct intel_atomic_state *state,
>  	/* step5: program and powerup panel */
>  	gen11_dsi_powerup_panel(encoder);
>  
> -	intel_dsc_enable(encoder, pipe_config);
> +	intel_dsc_dsi_pps_write(encoder, pipe_config);
> +
> +	intel_dsc_enable(pipe_config);
>  
>  	/* step6c: configure transcoder timings */
>  	gen11_dsi_set_transcoder_timings(encoder, pipe_config);
> @@ -1636,7 +1638,7 @@ static int gen11_dsi_dsc_compute_config(struct intel_encoder *encoder,
>  	/* FIXME: initialize from VBT */
>  	vdsc_cfg->rc_model_size = DSC_RC_MODEL_SIZE_CONST;
>  
> -	ret = intel_dsc_compute_params(encoder, crtc_state);
> +	ret = intel_dsc_compute_params(crtc_state);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4fbffce501dc..f51c5d732d41 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2434,7 +2434,10 @@ static void dg2_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  
>  	/* 5.k Configure and enable FEC if needed */
>  	intel_ddi_enable_fec(encoder, crtc_state);
> -	intel_dsc_enable(encoder, crtc_state);
> +
> +	intel_dsc_dp_pps_write(encoder, crtc_state);
> +
> +	intel_dsc_enable(crtc_state);
>  }
>  
>  static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
> @@ -2575,8 +2578,11 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  
>  	/* 7.l Configure and enable FEC if needed */
>  	intel_ddi_enable_fec(encoder, crtc_state);
> +
> +	intel_dsc_dp_pps_write(encoder, crtc_state);
> +
>  	if (!crtc_state->bigjoiner)
> -		intel_dsc_enable(encoder, crtc_state);
> +		intel_dsc_enable(crtc_state);
>  }
>  
>  static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
> @@ -2641,8 +2647,10 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	if (!is_mst)
>  		intel_ddi_enable_pipe_clock(encoder, crtc_state);
>  
> +	intel_dsc_dp_pps_write(encoder, crtc_state);
> +
>  	if (!crtc_state->bigjoiner)
> -		intel_dsc_enable(encoder, crtc_state);
> +		intel_dsc_enable(crtc_state);
>  }
>  
>  static void intel_ddi_pre_enable_dp(struct intel_atomic_state *state,
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 5716085e15f5..4e659a103984 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -3007,7 +3007,7 @@ static void icl_ddi_bigjoiner_pre_enable(struct intel_atomic_state *state,
>  
>  	if (!crtc_state->bigjoiner_slave) {
>  		/* need to enable VDSC, which we skipped in pre-enable */
> -		intel_dsc_enable(encoder, crtc_state);
> +		intel_dsc_enable(crtc_state);
>  	} else {
>  		/*
>  		 * Enable sequence steps 1-7 on bigjoiner master
> @@ -3017,8 +3017,7 @@ static void icl_ddi_bigjoiner_pre_enable(struct intel_atomic_state *state,
>  			intel_enable_shared_dpll(master_crtc_state);
>  		intel_encoders_pre_enable(state, master_crtc);
>  
> -		/* and DSC on slave */
> -		intel_dsc_enable(NULL, crtc_state);
> +		intel_dsc_enable(crtc_state);
>  	}
>  
>  	if (DISPLAY_VER(dev_priv) >= 13)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index e898834cc5f9..68ddd9a490e2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1260,7 +1260,7 @@ static int intel_dp_dsc_compute_params(struct intel_encoder *encoder,
>  	else
>  		vdsc_cfg->slice_height = 2;
>  
> -	ret = intel_dsc_compute_params(encoder, crtc_state);
> +	ret = intel_dsc_compute_params(crtc_state);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 2275f99ce9d7..fa84be609d5d 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -442,10 +442,10 @@ calculate_rc_params(struct rc_parameters *rc,
>  	}
>  }
>  
> -int intel_dsc_compute_params(struct intel_encoder *encoder,
> -			     struct intel_crtc_state *pipe_config)
> +int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	struct drm_dsc_config *vdsc_cfg = &pipe_config->dsc.config;
>  	u16 compressed_bpp = pipe_config->dsc.compressed_bpp;
>  	const struct rc_parameters *rc_params;
> @@ -1055,8 +1055,8 @@ static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
>  	}
>  }
>  
> -static void intel_dsc_dsi_pps_write(struct intel_encoder *encoder,
> -				    const struct intel_crtc_state *crtc_state)
> +void intel_dsc_dsi_pps_write(struct intel_encoder *encoder,
> +			     const struct intel_crtc_state *crtc_state)
>  {
>  	const struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config;
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> @@ -1064,6 +1064,9 @@ static void intel_dsc_dsi_pps_write(struct intel_encoder *encoder,
>  	struct drm_dsc_picture_parameter_set pps;
>  	enum port port;
>  
> +	if (!crtc_state->dsc.compression_enable)
> +		return;
> +
>  	drm_dsc_pps_payload_pack(&pps, vdsc_cfg);
>  
>  	for_each_dsi_port(port, intel_dsi->ports) {
> @@ -1074,14 +1077,16 @@ static void intel_dsc_dsi_pps_write(struct intel_encoder *encoder,
>  	}
>  }
>  
> -static void intel_dsc_dp_pps_write(struct intel_encoder *encoder,
> -				   const struct intel_crtc_state *crtc_state)
> +void intel_dsc_dp_pps_write(struct intel_encoder *encoder,
> +			    const struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> -	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>  	const struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config;
>  	struct drm_dsc_pps_infoframe dp_dsc_pps_sdp;
>  
> +	if (!crtc_state->dsc.compression_enable)
> +		return;
> +
>  	/* Prepare DP SDP PPS header as per DP 1.4 spec, Table 2-123 */
>  	drm_dsc_dp_pps_header_init(&dp_dsc_pps_sdp.pps_header);
>  
> @@ -1142,8 +1147,7 @@ void intel_uncompressed_joiner_enable(const struct intel_crtc_state *crtc_state)
>  	}
>  }
>  
> -void intel_dsc_enable(struct intel_encoder *encoder,
> -		      const struct intel_crtc_state *crtc_state)
> +void intel_dsc_enable(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -1155,13 +1159,6 @@ void intel_dsc_enable(struct intel_encoder *encoder,
>  
>  	intel_dsc_pps_configure(crtc_state);
>  
> -	if (!crtc_state->bigjoiner_slave) {
> -		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
> -			intel_dsc_dsi_pps_write(encoder, crtc_state);
> -		else
> -			intel_dsc_dp_pps_write(encoder, crtc_state);
> -	}
> -
>  	dss_ctl2_val |= LEFT_BRANCH_VDSC_ENABLE;
>  	if (crtc_state->dsc.dsc_split) {
>  		dss_ctl2_val |= RIGHT_BRANCH_VDSC_ENABLE;
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.h b/drivers/gpu/drm/i915/display/intel_vdsc.h
> index 0c5d80a572da..4ec75f715986 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.h
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.h
> @@ -15,15 +15,17 @@ struct intel_encoder;
>  
>  bool intel_dsc_source_support(const struct intel_crtc_state *crtc_state);
>  void intel_uncompressed_joiner_enable(const struct intel_crtc_state *crtc_state);
> -void intel_dsc_enable(struct intel_encoder *encoder,
> -		      const struct intel_crtc_state *crtc_state);
> +void intel_dsc_enable(const struct intel_crtc_state *crtc_state);
>  void intel_dsc_disable(const struct intel_crtc_state *crtc_state);
> -int intel_dsc_compute_params(struct intel_encoder *encoder,
> -			     struct intel_crtc_state *pipe_config);
> +int intel_dsc_compute_params(struct intel_crtc_state *pipe_config);
>  void intel_uncompressed_joiner_get_config(struct intel_crtc_state *crtc_state);
>  void intel_dsc_get_config(struct intel_crtc_state *crtc_state);
>  enum intel_display_power_domain
>  intel_dsc_power_domain(struct intel_crtc *crtc, enum transcoder cpu_transcoder);
>  struct intel_crtc *intel_dsc_get_bigjoiner_secondary(const struct intel_crtc *primary_crtc);
> +void intel_dsc_dsi_pps_write(struct intel_encoder *encoder,
> +			     const struct intel_crtc_state *crtc_state);
> +void intel_dsc_dp_pps_write(struct intel_encoder *encoder,
> +			    const struct intel_crtc_state *crtc_state);
>  
>  #endif /* __INTEL_VDSC_H__ */
> -- 
> 2.32.0
> 



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux