* [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
@ 2025-02-27 9:53 Suraj Kandpal
2025-02-27 10:42 ` Jani Nikula
0 siblings, 1 reply; 12+ messages in thread
From: Suraj Kandpal @ 2025-02-27 9:53 UTC (permalink / raw)
To: intel-xe, intel-gfx
Cc: ankit.k.nautiyal, uma.shankar, william.tseng, Suraj Kandpal
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)) {
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);
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
2025-02-27 9:53 [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels Suraj Kandpal
@ 2025-02-27 10:42 ` Jani Nikula
2025-02-27 10:45 ` Kandpal, Suraj
0 siblings, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2025-02-27 10:42 UTC (permalink / raw)
To: Suraj Kandpal, intel-xe, intel-gfx
Cc: ankit.k.nautiyal, uma.shankar, william.tseng, Suraj Kandpal
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);
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
2025-02-27 10:42 ` Jani Nikula
@ 2025-02-27 10:45 ` Kandpal, Suraj
0 siblings, 0 replies; 12+ messages in thread
From: Kandpal, Suraj @ 2025-02-27 10:45 UTC (permalink / raw)
To: Jani Nikula, intel-xe@lists.freedesktop.org,
intel-gfx@lists.freedesktop.org
Cc: Nautiyal, Ankit K, Shankar, Uma, Tseng, William
> -----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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
@ 2025-02-27 10:49 Suraj Kandpal
0 siblings, 0 replies; 12+ messages in thread
From: Suraj Kandpal @ 2025-02-27 10:49 UTC (permalink / raw)
To: intel-xe, intel-gfx
Cc: ankit.k.nautiyal, uma.shankar, william.tseng, jani.nikula,
Suraj Kandpal
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.
--v2
-Use intel_crtc_has_type [Jani]
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
drivers/gpu/drm/i915/display/intel_vdsc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
index 6e7151346382..affe9913f1ee 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -320,7 +320,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 ||
+ intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI))) {
calculate_rc_params(vdsc_cfg);
} else {
if ((compressed_bpp == 8 ||
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
@ 2025-02-27 11:26 Suraj Kandpal
2025-02-28 4:37 ` Nautiyal, Ankit K
0 siblings, 1 reply; 12+ messages in thread
From: Suraj Kandpal @ 2025-02-27 11:26 UTC (permalink / raw)
To: intel-xe, intel-gfx
Cc: ankit.k.nautiyal, uma.shankar, william.tseng, jani.nikula,
Suraj Kandpal
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.
--v2
-Use intel_crtc_has_type [Jani]
--v3
-Add Signed-off-by from William too [Ankit]
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Signed-off-by: William Tseng <william.tseng@intel.com>
---
drivers/gpu/drm/i915/display/intel_vdsc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
index 6e7151346382..affe9913f1ee 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -320,7 +320,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 ||
+ intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI))) {
calculate_rc_params(vdsc_cfg);
} else {
if ((compressed_bpp == 8 ||
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
2025-02-27 11:26 Suraj Kandpal
@ 2025-02-28 4:37 ` Nautiyal, Ankit K
2025-02-28 4:47 ` Kandpal, Suraj
0 siblings, 1 reply; 12+ messages in thread
From: Nautiyal, Ankit K @ 2025-02-28 4:37 UTC (permalink / raw)
To: Suraj Kandpal, intel-xe, intel-gfx
Cc: uma.shankar, william.tseng, jani.nikula
On 2/27/2025 4:56 PM, Suraj Kandpal 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.
>
> --v2
> -Use intel_crtc_has_type [Jani]
>
> --v3
> -Add Signed-off-by from William too [Ankit]
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: William Tseng <william.tseng@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_vdsc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 6e7151346382..affe9913f1ee 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -320,7 +320,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 ||
> + intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI))) {
This should be !intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI)
I think it would be better to use a function for special handling for
DSI panel with DSC1.1.
(I am not very sure what should be an appropriate name for this but just
to give an example)
bool is_mipi_dsi_dsc_1_1()
{
return vdsc_cfg->dsc_version_minor == 1 &&
intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI);
}
The condition will then become:
if (DISPLAY_VER(dev_priv) >= 13 && !is_mipi_dsi_dsc_1_1())
calculate_rc_params(vdsc_cfg);
With this we also need to document about why we are not using calculate_rc_params for MIPI DSI with DSC1.1 in comment above the function.
Regards,
Ankit
> calculate_rc_params(vdsc_cfg);
> } else {
> if ((compressed_bpp == 8 ||
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
2025-02-28 4:37 ` Nautiyal, Ankit K
@ 2025-02-28 4:47 ` Kandpal, Suraj
0 siblings, 0 replies; 12+ messages in thread
From: Kandpal, Suraj @ 2025-02-28 4:47 UTC (permalink / raw)
To: Nautiyal, Ankit K, intel-xe@lists.freedesktop.org,
intel-gfx@lists.freedesktop.org
Cc: Shankar, Uma, Tseng, William, Nikula, Jani
> Subject: Re: [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
>
>
> On 2/27/2025 4:56 PM, Suraj Kandpal 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.
> >
> > --v2
> > -Use intel_crtc_has_type [Jani]
> >
> > --v3
> > -Add Signed-off-by from William too [Ankit]
> >
> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > Signed-off-by: William Tseng <william.tseng@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_vdsc.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > index 6e7151346382..affe9913f1ee 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > @@ -320,7 +320,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 ||
> > + intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI))) {
>
> This should be !intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI)
>
> I think it would be better to use a function for special handling for DSI panel
> with DSC1.1.
>
> (I am not very sure what should be an appropriate name for this but just to give
> an example)
>
> bool is_mipi_dsi_dsc_1_1()
Since this is a static function I think this naming should work
> {
> return vdsc_cfg->dsc_version_minor == 1 &&
> intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI); }
>
> The condition will then become:
>
> if (DISPLAY_VER(dev_priv) >= 13 && !is_mipi_dsi_dsc_1_1())
> calculate_rc_params(vdsc_cfg);
>
>
> With this we also need to document about why we are not using
> calculate_rc_params for MIPI DSI with DSC1.1 in comment above the function.
>
Sure will add needed documentation
Regards,
Suraj Kandpal
> Regards,
> Ankit
>
> > calculate_rc_params(vdsc_cfg);
> > } else {
> > if ((compressed_bpp == 8 ||
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
@ 2025-02-28 1:15 Yu, Gareth
0 siblings, 0 replies; 12+ messages in thread
From: Yu, Gareth @ 2025-02-28 1:15 UTC (permalink / raw)
To: Kandpal, Suraj
Cc: Nautiyal, Ankit K, intel-gfx@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, Nikula, Jani, Shankar, Uma,
Tseng, William
> From mboxrd@z Thu Jan 1 00:00:00 1970
> Return-Path: mailto:intel-xe-bounces@lists.freedesktop.org
> X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
> aws-us-west-2-korg-lkml-1.web.codeaurora.org
> Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177])
> (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
> (No client certificate requested)
> by smtp.lore.kernel.org (Postfix) with ESMTPS id E3D23C19F2E
> for mailto:intel-xe@archiver.kernel.org; Thu, 27 Feb 2025 11:26:56 +0000 (UTC)
> Received: from gabe.freedesktop.org (localhost [127.0.0.1])
> by gabe.freedesktop.org (Postfix) with ESMTP id A7C1C10EAB7;
> Thu, 27 Feb 2025 11:26:56 +0000 (UTC)
> Authentication-Results: gabe.freedesktop.org;
> dkim=pass (2048-bit key; unprotected) header.d=intel.com mailto:header.i=@intel.com header.b="MY6Bt0hC";
> dkim-atps=neutral
> Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.13])
> by gabe.freedesktop.org (Postfix) with ESMTPS id A87DB10EA9E;
> Thu, 27 Feb 2025 11:26:55 +0000 (UTC)
> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple;
> d=intel.com; mailto:i=@intel.com; q=dns/txt; s=Intel;
> t=1740655616; x=1772191616;
> h=from:to:cc:subject:date:message-id:mime-version:
> content-transfer-encoding;
> bh=8+o+KH5SGXJL9pbefqym6H7zNpSc5021IPN/PdKwNsU=;
> b=MY6Bt0hCjt+GFDT82nKYuYZ8t3ShCcp4URQO/YXiWFVIhZbAZpbiMYzZ
> dpzC9aG2zCv1V3KglhDw6BgmJjGcnY/GDf0KD0me4MN1098RLXCbY3Zlo
> g68++M+uiF699gmgAasdx1BWFkcYysaszs8DBcyyFE7zNw7OM9f6Q5dzD
> VPmG6XxflXUYU7Mpij3Wn8vBQ9DPqwXzbPy1Iv5Ojis2Rb5CmpnP82HWK
> SJU6CUlgcuF1b7d0XTyBkYaQ/KjmdRVNsHW8cSMLvpzum54UtY2cmsrM3
> dCeE+isV4RtwPDaWmX1SeEF9F7Y4cL+9vDj4NTM2INZqCfAY+UCM4aKiZ Q==;
> X-CSE-ConnectionGUID: gR/d5ikPS82kkQ2JSyF2KA==
> X-CSE-MsgGUID: GUC8x5VDRo+UNdtKbxA6/Q==
> X-IronPort-AV: E=McAfee;i="6700,10204,11357"; a="52527215"
> X-IronPort-AV: E=Sophos;i="6.13,319,1732608000"; d="scan'208";a="52527215"
> Received: from orviesa001.jf.intel.com ([10.64.159.141])
> by orvoesa105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
> 27 Feb 2025 03:26:56 -0800
> X-CSE-ConnectionGUID: mUJY05jTSP2Aj3LHisZ5pw==
> X-CSE-MsgGUID: TtB54KpmQjOcHenFYonybQ==
> X-ExtLoop1: 1
> X-IronPort-AV: E=Sophos;i="6.12,224,1728975600"; d="scan'208";a="154180517"
> Received: from kandpal-x299-ud4-pro.iind.intel.com ([10.190.239.10])
> by orviesa001.jf.intel.com with ESMTP; 27 Feb 2025 03:26:53 -0800
> From: Suraj Kandpal mailto:suraj.kandpal@intel.com
> To: mailto:intel-xe@lists.freedesktop.org,
> mailto:intel-gfx@lists.freedesktop.org
> Cc: mailto:ankit.k.nautiyal@intel.com, mailto:uma.shankar@intel.com, mailto:william.tseng@intel.com,
> mailto:jani.nikula@intel.com, Suraj Kandpal mailto:suraj.kandpal@intel.com
> Subject: [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
> Date: Thu, 27 Feb 2025 16:56:54 +0530
> Message-Id: mailto:20250227112654.279160-1-suraj.kandpal@intel.com
> X-Mailer: git-send-email 2.34.1
> MIME-Version: 1.0
> Content-Transfer-Encoding: 8bit
> X-BeenThere: mailto:intel-xe@lists.freedesktop.org
> X-Mailman-Version: 2.1.29
> Precedence: list
> List-Id: Intel Xe graphics driver <intel-xe.lists.freedesktop.org>
> List-Unsubscribe: https://lists.freedesktop.org/mailman/options/intel-xe,
> mailto:intel-xe-request@lists.freedesktop.org?subject=unsubscribe
> List-Archive: https://lists.freedesktop.org/archives/intel-xe
> List-Post: mailto:intel-xe@lists.freedesktop.org
> List-Help: mailto:intel-xe-request@lists.freedesktop.org?subject=help
> List-Subscribe: https://lists.freedesktop.org/mailman/listinfo/intel-xe,
> mailto:intel-xe-request@lists.freedesktop.org?subject=subscribe
> Errors-To: mailto:intel-xe-bounces@lists.freedesktop.org
> Sender: "Intel-xe" mailto:intel-xe-bounces@lists.freedesktop.org
>
> 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.
>
> --v2
> -Use intel_crtc_has_type [Jani]
>
> --v3
> -Add Signed-off-by from William too [Ankit]
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
> Signed-off-by: Suraj Kandpal mailto:suraj.kandpal@intel.com
> Signed-off-by: William Tseng mailto:william.tseng@intel.com
> ---
> drivers/gpu/drm/i915/display/intel_vdsc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 6e7151346382..affe9913f1ee 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -320,7 +320,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 &&
Please change to "if (DISPLAY_VER(dev_priv) >= 14 &&" because MTL begins to support DSC 1.2a according bspec 49259.
> + (vdsc_cfg->dsc_version_minor != 1 ||
> + intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI))) {
> calculate_rc_params(vdsc_cfg);
> } else {
> if ((compressed_bpp == 8 ||
> --
> 2.34.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
@ 2025-02-28 5:10 Suraj Kandpal
2025-02-28 9:15 ` Jani Nikula
0 siblings, 1 reply; 12+ messages in thread
From: Suraj Kandpal @ 2025-02-28 5:10 UTC (permalink / raw)
To: intel-xe, intel-gfx
Cc: ankit.k.nautiyal, uma.shankar, william.tseng, jani.nikula,
Suraj Kandpal
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.
--v2
-Use intel_crtc_has_type [Jani]
--v3
-Add Signed-off-by from William too [Ankit]
--v4
-Use a function to check Mipi dsi dsc 1.1 condition [Ankit]
-Add documentation for using this condition [Ankit]
-Rebase
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Signed-off-by: William Tseng <william.tseng@intel.com>
---
drivers/gpu/drm/i915/display/intel_vdsc.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
index 3ed64c17bdff..2a6706517cfa 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -259,6 +259,13 @@ static int intel_dsc_slice_dimensions_valid(struct intel_crtc_state *pipe_config
return 0;
}
+static bool is_mipi_dsi_dsc_1_1(struct drm_dsc_config *vdsc_cfg,
+ struct intel_crtc_state *crtc_state)
+{
+ return vdsc_cfg->dsc_version_minor == 1 &&
+ intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI);
+}
+
int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
{
struct intel_display *display = to_intel_display(pipe_config);
@@ -317,8 +324,14 @@ int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
* From XE_LPD onwards we supports compression bpps in steps of 1
* upto uncompressed bpp-1, hence add calculations for all the rc
* parameters
+ * We also don't want to calculate all rc parameters when the panel
+ * is MIPI DSI and it's using DSC 1.1. The reason being that some
+ * DSI panels vendors have hardcoded PPS params in the VBT causing
+ * the parameters sent from the source to be ignore. This causes a
+ * noise in the display.
*/
- if (DISPLAY_VER(display) >= 13) {
+ if (DISPLAY_VER(display) >= 13 &&
+ !is_mipi_dsi_dsc_1_1(vdsc_cfg, pipe_config)) {
calculate_rc_params(vdsc_cfg);
} else {
if ((compressed_bpp == 8 ||
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
2025-02-28 5:10 Suraj Kandpal
@ 2025-02-28 9:15 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2025-02-28 9:15 UTC (permalink / raw)
To: Suraj Kandpal, intel-xe, intel-gfx
Cc: ankit.k.nautiyal, uma.shankar, william.tseng, Suraj Kandpal
On Fri, 28 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.
>
> --v2
> -Use intel_crtc_has_type [Jani]
>
> --v3
> -Add Signed-off-by from William too [Ankit]
Why?
People seem to assign all sorts of meanings to Signed-off-by, but it's
really about certifying [1]. It's certainly not about giving credits.
[1] https://developercertificate.org/
>
> --v4
> -Use a function to check Mipi dsi dsc 1.1 condition [Ankit]
> -Add documentation for using this condition [Ankit]
> -Rebase
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: William Tseng <william.tseng@intel.com>
If William was indeed involved in writing the patch, his Signed-off-by,
possibly with a Co-developed-by, needs to come first. The subsequent
handlers add their Signed-off-by at the end. For instance, if I applied
the patch, my Signed-off-by would be added at the end.
> ---
> drivers/gpu/drm/i915/display/intel_vdsc.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 3ed64c17bdff..2a6706517cfa 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -259,6 +259,13 @@ static int intel_dsc_slice_dimensions_valid(struct intel_crtc_state *pipe_config
> return 0;
> }
>
> +static bool is_mipi_dsi_dsc_1_1(struct drm_dsc_config *vdsc_cfg,
> + struct intel_crtc_state *crtc_state)
We don't really use "mipi_dsi" naming, just like we don't use
"vesa_dp". It's just DSI, and it's clear enough.
There's no need to pass vdsc_cfg separately. You can figure it out from
crtc_state.
> +{
> + return vdsc_cfg->dsc_version_minor == 1 &&
I wonder how many places would blow up with the addition of DSC 2.1...
This isn't the only place that gets this wrong, but we really need to be
more careful about the DSC version checks. Might as well start with new
code being added.
> + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI);
> +}
> +
> int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
> {
> struct intel_display *display = to_intel_display(pipe_config);
> @@ -317,8 +324,14 @@ int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
> * From XE_LPD onwards we supports compression bpps in steps of 1
> * upto uncompressed bpp-1, hence add calculations for all the rc
> * parameters
Add a blank comment line here to separate paragraphs.
> + * We also don't want to calculate all rc parameters when the panel
> + * is MIPI DSI and it's using DSC 1.1. The reason being that some
> + * DSI panels vendors have hardcoded PPS params in the VBT causing
> + * the parameters sent from the source to be ignore. This causes a
> + * noise in the display.
> */
> - if (DISPLAY_VER(display) >= 13) {
> + if (DISPLAY_VER(display) >= 13 &&
> + !is_mipi_dsi_dsc_1_1(vdsc_cfg, pipe_config)) {
With the parameter removed, this'll probably fit on one line.
BR,
Jani.
> calculate_rc_params(vdsc_cfg);
> } else {
> if ((compressed_bpp == 8 ||
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
@ 2025-02-28 15:25 Suraj Kandpal
2025-03-11 5:33 ` Nautiyal, Ankit K
0 siblings, 1 reply; 12+ messages in thread
From: Suraj Kandpal @ 2025-02-28 15:25 UTC (permalink / raw)
To: intel-xe, intel-gfx
Cc: ankit.k.nautiyal, uma.shankar, william.tseng, jani.nikula,
Suraj Kandpal
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.
--v2
-Use intel_crtc_has_type [Jani]
--v4
-Use a function to check Mipi dsi dsc 1.1 condition [Ankit]
-Add documentation for using this condition [Ankit]
-Rebase
--v5
-Pass only the crtc_state [Jani]
-Fixup the comment [Jani]
-Check for dsc major version [Jani]
-Use co-developed-by tag [Jani]
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
Co-developed-by: William Tseng <william.tseng@intel.com>
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
drivers/gpu/drm/i915/display/intel_vdsc.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
index 3ed64c17bdff..04ba9f6b7ea2 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -259,6 +259,15 @@ static int intel_dsc_slice_dimensions_valid(struct intel_crtc_state *pipe_config
return 0;
}
+static bool is_dsi_dsc_1_1(struct intel_crtc_state *crtc_state)
+{
+ struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config;
+
+ return vdsc_cfg->dsc_version_major == 1 &&
+ vdsc_cfg->dsc_version_minor == 1 &&
+ intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI);
+}
+
int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
{
struct intel_display *display = to_intel_display(pipe_config);
@@ -317,8 +326,14 @@ int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
* From XE_LPD onwards we supports compression bpps in steps of 1
* upto uncompressed bpp-1, hence add calculations for all the rc
* parameters
+ *
+ * We also don't want to calculate all rc parameters when the panel
+ * is MIPI DSI and it's using DSC 1.1. The reason being that some
+ * DSI panels vendors have hardcoded PPS params in the VBT causing
+ * the parameters sent from the source to be ignore. This causes a
+ * noise in the display.
*/
- if (DISPLAY_VER(display) >= 13) {
+ if (DISPLAY_VER(display) >= 13 && !is_dsi_dsc_1_1(pipe_config)) {
calculate_rc_params(vdsc_cfg);
} else {
if ((compressed_bpp == 8 ||
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels
2025-02-28 15:25 Suraj Kandpal
@ 2025-03-11 5:33 ` Nautiyal, Ankit K
0 siblings, 0 replies; 12+ messages in thread
From: Nautiyal, Ankit K @ 2025-03-11 5:33 UTC (permalink / raw)
To: Suraj Kandpal, intel-xe, intel-gfx
Cc: uma.shankar, william.tseng, jani.nikula
On 2/28/2025 8:55 PM, Suraj Kandpal 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.
>
> --v2
> -Use intel_crtc_has_type [Jani]
>
> --v4
> -Use a function to check Mipi dsi dsc 1.1 condition [Ankit]
> -Add documentation for using this condition [Ankit]
> -Rebase
>
> --v5
> -Pass only the crtc_state [Jani]
> -Fixup the comment [Jani]
> -Check for dsc major version [Jani]
> -Use co-developed-by tag [Jani]
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
> Co-developed-by: William Tseng <william.tseng@intel.com>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_vdsc.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index 3ed64c17bdff..04ba9f6b7ea2 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -259,6 +259,15 @@ static int intel_dsc_slice_dimensions_valid(struct intel_crtc_state *pipe_config
> return 0;
> }
>
> +static bool is_dsi_dsc_1_1(struct intel_crtc_state *crtc_state)
> +{
> + struct drm_dsc_config *vdsc_cfg = &crtc_state->dsc.config;
> +
> + return vdsc_cfg->dsc_version_major == 1 &&
> + vdsc_cfg->dsc_version_minor == 1 &&
> + intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI);
> +}
> +
> int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
> {
> struct intel_display *display = to_intel_display(pipe_config);
> @@ -317,8 +326,14 @@ int intel_dsc_compute_params(struct intel_crtc_state *pipe_config)
> * From XE_LPD onwards we supports compression bpps in steps of 1
> * upto uncompressed bpp-1, hence add calculations for all the rc
> * parameters
> + *
> + * We also don't want to calculate all rc parameters when the panel
> + * is MIPI DSI and it's using DSC 1.1. The reason being that some
> + * DSI panels vendors have hardcoded PPS params in the VBT causing
> + * the parameters sent from the source to be ignore. This causes a
> + * noise in the display.
If I understand correctly, the issue is seen because in
calculate_rc_params() we are interpolating some of the params for
varying DSC variables like bits_per_pixel.
The derived params though based on the DSC1.2a C-Model, may have a bit
difference from the original parameters set which generally is fine and
is known to be working.
However for some DSI panels that support DSC1.1, few of the PPS params
are hard coded. Due to this, there is some mismatch with derived rc
parameters with the parameters the panel expects.
Furthermore for DSI panels we are currently using bits_per_pixel
(compressed bpp) hardcoded from VBT, (unlike other encoders where we
find the optimum compressed bpp) so dont need to rely on interpolation,
as we can get the required rc parameters from the tables.
So for such a case it makes sense to avoid using calculate_rc_params and
fallback to the specific rc parameters from the table.
Perhaps we can document some of this as this is not vert explicit from
comment.
In any case the change looks good to me.
Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> */
> - if (DISPLAY_VER(display) >= 13) {
> + if (DISPLAY_VER(display) >= 13 && !is_dsi_dsc_1_1(pipe_config)) {
> calculate_rc_params(vdsc_cfg);
> } else {
> if ((compressed_bpp == 8 ||
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-03-11 5:33 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-27 9:53 [PATCH] drm/i915/vdsc: Use the DSC config tables for DSI panels Suraj Kandpal
2025-02-27 10:42 ` Jani Nikula
2025-02-27 10:45 ` Kandpal, Suraj
-- strict thread matches above, loose matches on Subject: below --
2025-02-27 10:49 Suraj Kandpal
2025-02-27 11:26 Suraj Kandpal
2025-02-28 4:37 ` Nautiyal, Ankit K
2025-02-28 4:47 ` Kandpal, Suraj
2025-02-28 1:15 Yu, Gareth
2025-02-28 5:10 Suraj Kandpal
2025-02-28 9:15 ` Jani Nikula
2025-02-28 15:25 Suraj Kandpal
2025-03-11 5:33 ` Nautiyal, Ankit K
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox