Re: [Intel-gfx] [v4.1 08/16] drm/i915/dsc: add basic hardware state readout support

[Manasi Navare <manasi.d.navare@intel.com>]

On Tue, Dec 10, 2019 at 12:50:52PM +0200, Jani Nikula wrote:
> Add basic hardware state readout for DSC, and check the most relevant
> details in the state checker.
> 
> v2:
> - check for DSC power before reading its state
> - check if source supports DSC at all
> 
> As a side effect, this should also get the power domains for the enabled
> DSC on takeover, and subsequently disable DSC if it's not needed.
> 
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

This looks great, thank you for adding the readout. And we should
add more DSC parameters in the readout soon like Ville had suggested
adding all the PPS readout, but for now:

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

Manasi

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     |  2 +
>  drivers/gpu/drm/i915/display/intel_display.c |  4 ++
>  drivers/gpu/drm/i915/display/intel_vdsc.c    | 49 ++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_vdsc.h    |  2 +
>  4 files changed, 57 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 3e81c54c349e..5b6f32517c75 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4294,6 +4294,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  	if (WARN_ON(transcoder_is_dsi(cpu_transcoder)))
>  		return;
>  
> +	intel_dsc_get_config(encoder, pipe_config);
> +
>  	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>  	if (temp & TRANS_DDI_PHSYNC)
>  		flags |= DRM_MODE_FLAG_PHSYNC;
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 1d2fad1610ef..5a4bd37863e3 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13301,6 +13301,10 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  	PIPE_CONF_CHECK_I(sync_mode_slaves_mask);
>  	PIPE_CONF_CHECK_I(master_transcoder);
>  
> +	PIPE_CONF_CHECK_I(dsc.compression_enable);
> +	PIPE_CONF_CHECK_I(dsc.dsc_split);
> +	PIPE_CONF_CHECK_I(dsc.compressed_bpp);
> +
>  #undef PIPE_CONF_CHECK_X
>  #undef PIPE_CONF_CHECK_I
>  #undef PIPE_CONF_CHECK_BOOL
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index a1b0f7cf1a96..ed9048140937 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -864,6 +864,55 @@ static void intel_dsc_pps_configure(struct intel_encoder *encoder,
>  	}
>  }
>  
> +void intel_dsc_get_config(struct intel_encoder *encoder,
> +			  struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config;
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +	enum pipe pipe = crtc->pipe;
> +	enum intel_display_power_domain power_domain;
> +	intel_wakeref_t wakeref;
> +	u32 dss_ctl1, dss_ctl2, val;
> +
> +	if (!intel_dsc_source_support(encoder, crtc_state))
> +		return;
> +
> +	power_domain = intel_dsc_power_domain(crtc_state);
> +
> +	wakeref = intel_display_power_get_if_enabled(dev_priv, power_domain);
> +	if (!wakeref)
> +		return;
> +
> +	if (crtc_state->cpu_transcoder == TRANSCODER_EDP) {
> +		dss_ctl1 = I915_READ(DSS_CTL1);
> +		dss_ctl2 = I915_READ(DSS_CTL2);
> +	} else {
> +		dss_ctl1 = I915_READ(ICL_PIPE_DSS_CTL1(pipe));
> +		dss_ctl2 = I915_READ(ICL_PIPE_DSS_CTL2(pipe));
> +	}
> +
> +	crtc_state->dsc.compression_enable = dss_ctl2 & LEFT_BRANCH_VDSC_ENABLE;
> +	if (!crtc_state->dsc.compression_enable)
> +		goto out;
> +
> +	crtc_state->dsc.dsc_split = (dss_ctl2 & RIGHT_BRANCH_VDSC_ENABLE) &&
> +		(dss_ctl1 & JOINER_ENABLE);
> +
> +	/* FIXME: add more state readout as needed */
> +
> +	/* PPS1 */
> +	if (cpu_transcoder == TRANSCODER_EDP)
> +		val = I915_READ(DSCA_PICTURE_PARAMETER_SET_1);
> +	else
> +		val = I915_READ(ICL_DSC0_PICTURE_PARAMETER_SET_1(pipe));
> +	vdsc_cfg->bits_per_pixel = val;
> +	crtc_state->dsc.compressed_bpp = vdsc_cfg->bits_per_pixel >> 4;
> +out:
> +	intel_display_power_put(dev_priv, power_domain, wakeref);
> +}
> +
>  static void intel_dsc_dsi_pps_write(struct intel_encoder *encoder,
>  				    const struct intel_crtc_state *crtc_state)
>  {
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.h b/drivers/gpu/drm/i915/display/intel_vdsc.h
> index e6e9f5b5c6ff..4dd6bbf35e42 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.h
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.h
> @@ -16,6 +16,8 @@ void intel_dsc_enable(struct intel_encoder *encoder,
>  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);
> +void intel_dsc_get_config(struct intel_encoder *encoder,
> +			  struct intel_crtc_state *crtc_state);
>  enum intel_display_power_domain
>  intel_dsc_power_domain(const struct intel_crtc_state *crtc_state);
>  
> -- 
> 2.20.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx