[v4.1,08/16] drm/i915/dsc: add basic hardware state readout support
diff mbox series

Message ID 3fb018cf9bd9a4c275aab389b6ec0f2a4e938bb9.1575974743.git.jani.nikula@intel.com
State New
Headers show
Series
  • drm/i915/dsi: enable DSC
Related show

Commit Message

Jani Nikula Dec. 10, 2019, 10:50 a.m. UTC
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>
---
 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(+)

Comments

Manasi Navare Dec. 10, 2019, 11:13 p.m. UTC | #1
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
>
Jani Nikula Dec. 11, 2019, 6:39 a.m. UTC | #2
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

Patch
diff mbox series

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