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

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 10 Dec 2019, Manasi Navare <manasi.d.navare@intel.com> wrote:
> 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:

Agreed, this is the start.

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

Thanks for the reviews!

BR,
Jani.

>
> 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

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx