Re: [Intel-gfx] [PATCH v8 13/19] drm/i915/dp: Configure i915 Picture parameter Set registers during DSC enabling
On Tue, Nov 06, 2018 at 04:36:08PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 02, 2018 at 02:31:32PM -0700, Manasi Navare wrote:
> > After encoder->pre_enable() hook, after link training sequence is
> > completed, PPS registers for DSC encoder are configured using the
> > DSC state parameters in intel_crtc_state as part of DSC enabling
> > routine in the source. DSC enabling routine is called after
> > encoder->pre_enable() before enbaling the pipe and after
> > compression is enabled on the sink.
> >
> > v6:
> > intel_dsc_enable to be part of pre_enable hook (Ville)
> > v5:
> > * make crtc_state const (Ville)
> > v4:
> > * Use cpu_transcoder instead of encoder->type for using EDP transcoder
> > DSC registers(Ville)
> > * Keep all PSS regs together (Anusha)
> >
> > v3:
> > * Configure Pic_width/2 for each VDSC engine when two VDSC engines per pipe
> > are used (Manasi)
> > * Add DSC slice_row_per_frame in PPS16 (Manasi)
> >
> > v2:
> > * Enable PG2 power well for VDSC on eDP
> >
> > Cc: Jani Nikula
> > Cc: Ville Syrjala
> > Cc: Anusha Srivatsa
> > Signed-off-by: Manasi Navare
> > Reviewed-by: Anusha Srivatsa
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 2 +
> > drivers/gpu/drm/i915/intel_ddi.c | 6 +
> > drivers/gpu/drm/i915/intel_display.c | 1 +
> > drivers/gpu/drm/i915/intel_vdsc.c| 419 +++
> > 4 files changed, 428 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index d7797a41c648..f347d0d7b9eb 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3485,6 +3485,8 @@ extern void intel_rps_mark_interactive(struct
> > drm_i915_private *i915,
> >bool interactive);
> > extern bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
> > bool enable);
> > +extern void intel_dsc_enable(struct intel_encoder *encoder,
> > +const struct intel_crtc_state *crtc_state);
> >
> > int i915_reg_read_ioctl(struct drm_device *dev, void *data,
> > struct drm_file *file);
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index af12c15ed94f..bba08322afb7 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2962,6 +2962,12 @@ static void intel_ddi_pre_enable_dp(struct
> > intel_encoder *encoder,
> >
> > if (!is_mst)
> > intel_ddi_enable_pipe_clock(crtc_state);
> > +
> > + /*
> > +* Enable and Configure Display Stream Compression in the source
> > +* if enabled in intel_crtc_state.
> > +*/
> > + intel_dsc_enable(encoder, crtc_state);
> > }
> >
> > static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 477e53c37353..d3aa77f4d606 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5417,6 +5417,7 @@ static void intel_encoders_pre_enable(struct drm_crtc
> > *crtc,
> >
> > if (encoder->pre_enable)
> > encoder->pre_enable(encoder, crtc_state, conn_state);
> > +
>
> Bogus whitespace leftover. Please try to read through your own patches
> before sending them. That generally helps catch silly things like this.
>
Oh yes, I will fix this, actually I did but the new versions were sent with CI
tag after this to get the
CI results.
Manasi
> > }
> > }
> >
> > diff --git a/drivers/gpu/drm/i915/intel_vdsc.c
> > b/drivers/gpu/drm/i915/intel_vdsc.c
> > index a76f78b9c0ee..0eaa69778160 100644
> > --- a/drivers/gpu/drm/i915/intel_vdsc.c
> > +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> > @@ -580,3 +580,422 @@ int intel_dp_compute_dsc_params(struct intel_dp
> > *intel_dp,
> >
> > return 0;
> > }
> > +
> > +static void intel_configure_pps_for_dsc_encoder(struct intel_encoder
> > *encoder,
> > + const struct intel_crtc_state
> > *crtc_state)
> > +{
> > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > + const struct drm_dsc_config *vdsc_cfg = &crtc_state->dp_dsc_cfg;
> > + enum pipe pipe = crtc->pipe;
> > + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > + u32 pps_val = 0;
> > + u32 rc_buf_thresh_dword[4];
> > + u32 rc_range_params_dword[8];
> > + u8 num_vdsc_instances = (crtc_state->dsc_params.dsc_split) ? 2 : 1;
> > + int i = 0;
> > +
> > + /* Populate PICTURE_PARAMETER_SET_0 registers */
> > + pps_val = DSC_VER_MAJ | vdsc_cfg->dsc_version_minor <<
> > + DSC_VER_MIN_SHIFT |
> > + vdsc_cfg->bits_per_component << DSC_BPC_SHIFT |
> > + vdsc_cfg->line_buf_depth << DSC_LINE_BUF_DEPTH_SHIFT;
> > + if (vdsc_cfg->block_pre
Re: [Intel-gfx] [PATCH v8 13/19] drm/i915/dp: Configure i915 Picture parameter Set registers during DSC enabling
On Fri, Nov 02, 2018 at 02:31:32PM -0700, Manasi Navare wrote:
> After encoder->pre_enable() hook, after link training sequence is
> completed, PPS registers for DSC encoder are configured using the
> DSC state parameters in intel_crtc_state as part of DSC enabling
> routine in the source. DSC enabling routine is called after
> encoder->pre_enable() before enbaling the pipe and after
> compression is enabled on the sink.
>
> v6:
> intel_dsc_enable to be part of pre_enable hook (Ville)
> v5:
> * make crtc_state const (Ville)
> v4:
> * Use cpu_transcoder instead of encoder->type for using EDP transcoder
> DSC registers(Ville)
> * Keep all PSS regs together (Anusha)
>
> v3:
> * Configure Pic_width/2 for each VDSC engine when two VDSC engines per pipe
> are used (Manasi)
> * Add DSC slice_row_per_frame in PPS16 (Manasi)
>
> v2:
> * Enable PG2 power well for VDSC on eDP
>
> Cc: Jani Nikula
> Cc: Ville Syrjala
> Cc: Anusha Srivatsa
> Signed-off-by: Manasi Navare
> Reviewed-by: Anusha Srivatsa
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/intel_ddi.c | 6 +
> drivers/gpu/drm/i915/intel_display.c | 1 +
> drivers/gpu/drm/i915/intel_vdsc.c| 419 +++
> 4 files changed, 428 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d7797a41c648..f347d0d7b9eb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3485,6 +3485,8 @@ extern void intel_rps_mark_interactive(struct
> drm_i915_private *i915,
> bool interactive);
> extern bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
> bool enable);
> +extern void intel_dsc_enable(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state);
>
> int i915_reg_read_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index af12c15ed94f..bba08322afb7 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2962,6 +2962,12 @@ static void intel_ddi_pre_enable_dp(struct
> intel_encoder *encoder,
>
> if (!is_mst)
> intel_ddi_enable_pipe_clock(crtc_state);
> +
> + /*
> + * Enable and Configure Display Stream Compression in the source
> + * if enabled in intel_crtc_state.
> + */
> + intel_dsc_enable(encoder, crtc_state);
> }
>
> static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 477e53c37353..d3aa77f4d606 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5417,6 +5417,7 @@ static void intel_encoders_pre_enable(struct drm_crtc
> *crtc,
>
> if (encoder->pre_enable)
> encoder->pre_enable(encoder, crtc_state, conn_state);
> +
Bogus whitespace leftover. Please try to read through your own patches
before sending them. That generally helps catch silly things like this.
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_vdsc.c
> b/drivers/gpu/drm/i915/intel_vdsc.c
> index a76f78b9c0ee..0eaa69778160 100644
> --- a/drivers/gpu/drm/i915/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/intel_vdsc.c
> @@ -580,3 +580,422 @@ int intel_dp_compute_dsc_params(struct intel_dp
> *intel_dp,
>
> return 0;
> }
> +
> +static void intel_configure_pps_for_dsc_encoder(struct intel_encoder
> *encoder,
> + const struct intel_crtc_state
> *crtc_state)
> +{
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + const struct drm_dsc_config *vdsc_cfg = &crtc_state->dp_dsc_cfg;
> + enum pipe pipe = crtc->pipe;
> + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> + u32 pps_val = 0;
> + u32 rc_buf_thresh_dword[4];
> + u32 rc_range_params_dword[8];
> + u8 num_vdsc_instances = (crtc_state->dsc_params.dsc_split) ? 2 : 1;
> + int i = 0;
> +
> + /* Populate PICTURE_PARAMETER_SET_0 registers */
> + pps_val = DSC_VER_MAJ | vdsc_cfg->dsc_version_minor <<
> + DSC_VER_MIN_SHIFT |
> + vdsc_cfg->bits_per_component << DSC_BPC_SHIFT |
> + vdsc_cfg->line_buf_depth << DSC_LINE_BUF_DEPTH_SHIFT;
> + if (vdsc_cfg->block_pred_enable)
> + pps_val |= DSC_BLOCK_PREDICTION;
> + else
> + pps_val &= ~DSC_BLOCK_PREDICTION;
> + if (vdsc_cfg->convert_rgb)
> + pps_val |= DSC_COLOR_SPACE_CONVERSION;
> + else
> + pps_val &= ~DSC_COLOR_SPACE_CONVERSION;
> + if (vdsc_cfg->enable422)
> + pps_val |= DSC_422_ENABLE;
> + else
> +
[Intel-gfx] [PATCH v8 13/19] drm/i915/dp: Configure i915 Picture parameter Set registers during DSC enabling
After encoder->pre_enable() hook, after link training sequence is
completed, PPS registers for DSC encoder are configured using the
DSC state parameters in intel_crtc_state as part of DSC enabling
routine in the source. DSC enabling routine is called after
encoder->pre_enable() before enbaling the pipe and after
compression is enabled on the sink.
v6:
intel_dsc_enable to be part of pre_enable hook (Ville)
v5:
* make crtc_state const (Ville)
v4:
* Use cpu_transcoder instead of encoder->type for using EDP transcoder
DSC registers(Ville)
* Keep all PSS regs together (Anusha)
v3:
* Configure Pic_width/2 for each VDSC engine when two VDSC engines per pipe
are used (Manasi)
* Add DSC slice_row_per_frame in PPS16 (Manasi)
v2:
* Enable PG2 power well for VDSC on eDP
Cc: Jani Nikula
Cc: Ville Syrjala
Cc: Anusha Srivatsa
Signed-off-by: Manasi Navare
Reviewed-by: Anusha Srivatsa
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/intel_ddi.c | 6 +
drivers/gpu/drm/i915/intel_display.c | 1 +
drivers/gpu/drm/i915/intel_vdsc.c| 419 +++
4 files changed, 428 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d7797a41c648..f347d0d7b9eb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3485,6 +3485,8 @@ extern void intel_rps_mark_interactive(struct
drm_i915_private *i915,
bool interactive);
extern bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
bool enable);
+extern void intel_dsc_enable(struct intel_encoder *encoder,
+const struct intel_crtc_state *crtc_state);
int i915_reg_read_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index af12c15ed94f..bba08322afb7 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2962,6 +2962,12 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder
*encoder,
if (!is_mst)
intel_ddi_enable_pipe_clock(crtc_state);
+
+ /*
+* Enable and Configure Display Stream Compression in the source
+* if enabled in intel_crtc_state.
+*/
+ intel_dsc_enable(encoder, crtc_state);
}
static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 477e53c37353..d3aa77f4d606 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5417,6 +5417,7 @@ static void intel_encoders_pre_enable(struct drm_crtc
*crtc,
if (encoder->pre_enable)
encoder->pre_enable(encoder, crtc_state, conn_state);
+
}
}
diff --git a/drivers/gpu/drm/i915/intel_vdsc.c
b/drivers/gpu/drm/i915/intel_vdsc.c
index a76f78b9c0ee..0eaa69778160 100644
--- a/drivers/gpu/drm/i915/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/intel_vdsc.c
@@ -580,3 +580,422 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp,
return 0;
}
+
+static void intel_configure_pps_for_dsc_encoder(struct intel_encoder *encoder,
+ const struct intel_crtc_state
*crtc_state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ const struct drm_dsc_config *vdsc_cfg = &crtc_state->dp_dsc_cfg;
+ enum pipe pipe = crtc->pipe;
+ enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+ u32 pps_val = 0;
+ u32 rc_buf_thresh_dword[4];
+ u32 rc_range_params_dword[8];
+ u8 num_vdsc_instances = (crtc_state->dsc_params.dsc_split) ? 2 : 1;
+ int i = 0;
+
+ /* Populate PICTURE_PARAMETER_SET_0 registers */
+ pps_val = DSC_VER_MAJ | vdsc_cfg->dsc_version_minor <<
+ DSC_VER_MIN_SHIFT |
+ vdsc_cfg->bits_per_component << DSC_BPC_SHIFT |
+ vdsc_cfg->line_buf_depth << DSC_LINE_BUF_DEPTH_SHIFT;
+ if (vdsc_cfg->block_pred_enable)
+ pps_val |= DSC_BLOCK_PREDICTION;
+ else
+ pps_val &= ~DSC_BLOCK_PREDICTION;
+ if (vdsc_cfg->convert_rgb)
+ pps_val |= DSC_COLOR_SPACE_CONVERSION;
+ else
+ pps_val &= ~DSC_COLOR_SPACE_CONVERSION;
+ if (vdsc_cfg->enable422)
+ pps_val |= DSC_422_ENABLE;
+ else
+ pps_val &= ~DSC_422_ENABLE;
+ if (vdsc_cfg->vbr_enable)
+ pps_val |= DSC_VBR_ENABLE;
+ else
+ pps_val &= ~DSC_VBR_ENABLE;
+
+ DRM_INFO("PPS0 = 0x%08x\n", pps_val);
+ if (cpu_transcoder == TRANSCODER_EDP) {
+ I915_WRITE(DSCA_PICTURE_PARAMETER_SET_0, pps_val);
+ /*
+* If 2
