| Message ID | 20250227095337.263091-1-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/vdsc: Use the DSC config tables for DSI panels | expand |
On Thu, 27 Feb 2025, Suraj Kandpal <suraj.kandpal@intel.com> wrote: > Some DSI panel vendors end up hardcoding PPS params because of which > it does not listen to the params sent from the source. We use the > default config tables for DSI panels when using DSC 1.1 rather than > calculate our own rc parameters. > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719 > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/icl_dsi.c | 2 +- > drivers/gpu/drm/i915/display/intel_dp.c | 3 ++- > drivers/gpu/drm/i915/display/intel_vdsc.c | 7 +++++-- > drivers/gpu/drm/i915/display/intel_vdsc.h | 3 ++- > 4 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c > index 5d3d54922d62..9022692c86ef 100644 > --- a/drivers/gpu/drm/i915/display/icl_dsi.c > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c > @@ -1607,7 +1607,7 @@ static int gen11_dsi_dsc_compute_config(struct intel_encoder *encoder, > > vdsc_cfg->pic_height = crtc_state->hw.adjusted_mode.crtc_vdisplay; > > - ret = intel_dsc_compute_params(crtc_state); > + ret = intel_dsc_compute_params(crtc_state, encoder); > if (ret) > return ret; > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 03ca2e02ab02..14a8cdcd1762 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -1856,6 +1856,7 @@ static int intel_dp_dsc_compute_params(const struct intel_connector *connector, > { > struct intel_display *display = to_intel_display(connector); > struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config; > + struct intel_encoder *encoder = connector->encoder; > int ret; > > /* > @@ -1869,7 +1870,7 @@ static int intel_dp_dsc_compute_params(const struct intel_connector *connector, > > vdsc_cfg->slice_height = intel_dp_get_slice_height(vdsc_cfg->pic_height); > > - ret = intel_dsc_compute_params(crtc_state); > + ret = intel_dsc_compute_params(crtc_state, encoder); > 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 6e7151346382..fd8602c1fa7d 100644 > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > @@ -260,7 +260,8 @@ static int intel_dsc_slice_dimensions_valid(struct intel_crtc_state *pipe_config > return 0; > } > > -int intel_dsc_compute_params(struct intel_crtc_state *pipe_config) > +int intel_dsc_compute_params(struct intel_crtc_state *pipe_config, > + struct intel_encoder *encoder) > { > struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc); > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > @@ -320,7 +321,9 @@ int intel_dsc_compute_params(struct intel_crtc_state *pipe_config) > * upto uncompressed bpp-1, hence add calculations for all the rc > * parameters > */ > - if (DISPLAY_VER(dev_priv) >= 13) { > + if (DISPLAY_VER(dev_priv) >= 13 && > + (vdsc_cfg->dsc_version_minor != 1 || > + encoder->type != INTEL_OUTPUT_DSI)) { What's wrong with this: intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI) Saves the trouble of passing the encoder around. BR, Jani. > calculate_rc_params(vdsc_cfg); > } else { > if ((compressed_bpp == 8 || > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.h b/drivers/gpu/drm/i915/display/intel_vdsc.h > index 9e2812f99dd7..50681fb3c129 100644 > --- a/drivers/gpu/drm/i915/display/intel_vdsc.h > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.h > @@ -19,7 +19,8 @@ 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(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_crtc_state *pipe_config); > +int intel_dsc_compute_params(struct intel_crtc_state *pipe_config, > + struct intel_encoder *encoder); > 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);
> -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Thursday, February 27, 2025 4:13 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-xe@lists.freedesktop.org; > intel-gfx@lists.freedesktop.org > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Shankar, Uma > <uma.shankar@intel.com>; Tseng, William <william.tseng@intel.com>; > Kandpal, Suraj <suraj.kandpal@intel.com> > Subject: Re: [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels > > On Thu, 27 Feb 2025, Suraj Kandpal <suraj.kandpal@intel.com> wrote: > > Some DSI panel vendors end up hardcoding PPS params because of which > > it does not listen to the params sent from the source. We use the > > default config tables for DSI panels when using DSC 1.1 rather than > > calculate our own rc parameters. > > > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719 > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > drivers/gpu/drm/i915/display/icl_dsi.c | 2 +- > > drivers/gpu/drm/i915/display/intel_dp.c | 3 ++- > > drivers/gpu/drm/i915/display/intel_vdsc.c | 7 +++++-- > > drivers/gpu/drm/i915/display/intel_vdsc.h | 3 ++- > > 4 files changed, 10 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c > > b/drivers/gpu/drm/i915/display/icl_dsi.c > > index 5d3d54922d62..9022692c86ef 100644 > > --- a/drivers/gpu/drm/i915/display/icl_dsi.c > > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c > > @@ -1607,7 +1607,7 @@ static int gen11_dsi_dsc_compute_config(struct > > intel_encoder *encoder, > > > > vdsc_cfg->pic_height = crtc_state->hw.adjusted_mode.crtc_vdisplay; > > > > - ret = intel_dsc_compute_params(crtc_state); > > + ret = intel_dsc_compute_params(crtc_state, encoder); > > if (ret) > > return ret; > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index 03ca2e02ab02..14a8cdcd1762 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -1856,6 +1856,7 @@ static int intel_dp_dsc_compute_params(const > > struct intel_connector *connector, { > > struct intel_display *display = to_intel_display(connector); > > struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config; > > + struct intel_encoder *encoder = connector->encoder; > > int ret; > > > > /* > > @@ -1869,7 +1870,7 @@ static int intel_dp_dsc_compute_params(const > > struct intel_connector *connector, > > > > vdsc_cfg->slice_height = > > intel_dp_get_slice_height(vdsc_cfg->pic_height); > > > > - ret = intel_dsc_compute_params(crtc_state); > > + ret = intel_dsc_compute_params(crtc_state, encoder); > > 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 6e7151346382..fd8602c1fa7d 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > > @@ -260,7 +260,8 @@ static int intel_dsc_slice_dimensions_valid(struct > intel_crtc_state *pipe_config > > return 0; > > } > > > > -int intel_dsc_compute_params(struct intel_crtc_state *pipe_config) > > +int intel_dsc_compute_params(struct intel_crtc_state *pipe_config, > > + struct intel_encoder *encoder) > > { > > struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc); > > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ > > -320,7 +321,9 @@ int intel_dsc_compute_params(struct intel_crtc_state > *pipe_config) > > * upto uncompressed bpp-1, hence add calculations for all the rc > > * parameters > > */ > > - if (DISPLAY_VER(dev_priv) >= 13) { > > + if (DISPLAY_VER(dev_priv) >= 13 && > > + (vdsc_cfg->dsc_version_minor != 1 || > > + encoder->type != INTEL_OUTPUT_DSI)) { > > What's wrong with this: > > intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI) > > Saves the trouble of passing the encoder around. Sure will use that instead Regards, Suraj Kandpal > > BR, > Jani. > > > > calculate_rc_params(vdsc_cfg); > > } else { > > if ((compressed_bpp == 8 || > > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.h > > b/drivers/gpu/drm/i915/display/intel_vdsc.h > > index 9e2812f99dd7..50681fb3c129 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vdsc.h > > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.h > > @@ -19,7 +19,8 @@ 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(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_crtc_state > > *pipe_config); > > +int intel_dsc_compute_params(struct intel_crtc_state *pipe_config, > > + struct intel_encoder *encoder); > > 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); > > -- > Jani Nikula, Intel
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c index 5d3d54922d62..9022692c86ef 100644 --- a/drivers/gpu/drm/i915/display/icl_dsi.c +++ b/drivers/gpu/drm/i915/display/icl_dsi.c @@ -1607,7 +1607,7 @@ static int gen11_dsi_dsc_compute_config(struct intel_encoder *encoder, vdsc_cfg->pic_height = crtc_state->hw.adjusted_mode.crtc_vdisplay; - ret = intel_dsc_compute_params(crtc_state); + ret = intel_dsc_compute_params(crtc_state, encoder); if (ret) return ret; diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 03ca2e02ab02..14a8cdcd1762 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -1856,6 +1856,7 @@ static int intel_dp_dsc_compute_params(const struct intel_connector *connector, { struct intel_display *display = to_intel_display(connector); struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config; + struct intel_encoder *encoder = connector->encoder; int ret; /* @@ -1869,7 +1870,7 @@ static int intel_dp_dsc_compute_params(const struct intel_connector *connector, vdsc_cfg->slice_height = intel_dp_get_slice_height(vdsc_cfg->pic_height); - ret = intel_dsc_compute_params(crtc_state); + ret = intel_dsc_compute_params(crtc_state, encoder); 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 6e7151346382..fd8602c1fa7d 100644 --- a/drivers/gpu/drm/i915/display/intel_vdsc.c +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c @@ -260,7 +260,8 @@ static int intel_dsc_slice_dimensions_valid(struct intel_crtc_state *pipe_config return 0; } -int intel_dsc_compute_params(struct intel_crtc_state *pipe_config) +int intel_dsc_compute_params(struct intel_crtc_state *pipe_config, + struct intel_encoder *encoder) { struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc); struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -320,7 +321,9 @@ int intel_dsc_compute_params(struct intel_crtc_state *pipe_config) * upto uncompressed bpp-1, hence add calculations for all the rc * parameters */ - if (DISPLAY_VER(dev_priv) >= 13) { + if (DISPLAY_VER(dev_priv) >= 13 && + (vdsc_cfg->dsc_version_minor != 1 || + encoder->type != INTEL_OUTPUT_DSI)) { calculate_rc_params(vdsc_cfg); } else { if ((compressed_bpp == 8 || diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.h b/drivers/gpu/drm/i915/display/intel_vdsc.h index 9e2812f99dd7..50681fb3c129 100644 --- a/drivers/gpu/drm/i915/display/intel_vdsc.h +++ b/drivers/gpu/drm/i915/display/intel_vdsc.h @@ -19,7 +19,8 @@ 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(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_crtc_state *pipe_config); +int intel_dsc_compute_params(struct intel_crtc_state *pipe_config, + struct intel_encoder *encoder); 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);
Some DSI panel vendors end up hardcoding PPS params because of which it does not listen to the params sent from the source. We use the default config tables for DSI panels when using DSC 1.1 rather than calculate our own rc parameters. Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719 Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- drivers/gpu/drm/i915/display/icl_dsi.c | 2 +- drivers/gpu/drm/i915/display/intel_dp.c | 3 ++- drivers/gpu/drm/i915/display/intel_vdsc.c | 7 +++++-- drivers/gpu/drm/i915/display/intel_vdsc.h | 3 ++- 4 files changed, 10 insertions(+), 5 deletions(-)