diff mbox series

drm/i915/vdsc: Correct the conditions of DSC1.1 and DSC 1.2 for rc params calculation

Message ID 20250221041401.2219258-1-gareth.yu@intel.com (mailing list archive)
State New
Headers show
Series drm/i915/vdsc: Correct the conditions of DSC1.1 and DSC 1.2 for rc params calculation | expand

Commit Message

Yu, Gareth Feb. 21, 2025, 4:14 a.m. UTC 
From: Gareth Yu <gareth.yu@intel.com>

The condition change is because Gen 14 begins to support DSC 1.2
and need to check if the sink supports DSC1.2

Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719

Cc: Suraj Kandpal <suraj.kandpal@intel.com>
Cc: Juha-Pekka Heikkil <juha-pekka.heikkila@intel.com>
Signed-off-by: Gareth Yu <gareth.yu@intel.com>
---
 drivers/gpu/drm/i915/display/intel_vdsc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kandpal, Suraj Feb. 21, 2025, 4:43 a.m. UTC | #1 
> -----Original Message-----
> From: Yu, Gareth <gareth.yu@intel.com>
> Sent: Friday, February 21, 2025 9:44 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: intel-xe@lists.freedesktop.org; shawn.c.lee.intel.om; Tseng, William
> <william.tseng@intel.com>; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Yu,
> Gareth <gareth.yu@intel.com>; Kandpal, Suraj <suraj.kandpal@intel.com>;
> Heikkila, Juha-pekka <juha-pekka.heikkila@intel.com>
> Subject: [PATCH] drm/i915/vdsc: Correct the conditions of DSC1.1 and DSC 1.2
> for rc params calculation
> 
> From: Gareth Yu <gareth.yu@intel.com>
> 
> The condition change is because Gen 14 begins to support DSC 1.2 and need to
> check if the sink supports DSC1.2
> 
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
> 
> Cc: Suraj Kandpal <suraj.kandpal@intel.com>
> Cc: Juha-Pekka Heikkil <juha-pekka.heikkila@intel.com>
> Signed-off-by: Gareth Yu <gareth.yu@intel.com>

Why has the authorship and Signed-off-by changed for essentially the same patch.
Also the commit message and versioning information has been removed any changes
Needs to go as a new version to the previous patch.


> ---
>  drivers/gpu/drm/i915/display/intel_vdsc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index b355c479eda3..555180e40b72 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -320,7 +320,7 @@ 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) >= 14) && (vdsc_cfg->dsc_version_minor >=
> +2)) {

Please add a bspec reference number in the commit message where it says we started support of DSC 1.2 from MTL

Regards,
Suraj Kandpal

>  		calculate_rc_params(vdsc_cfg);
>  	} else {
>  		if ((compressed_bpp == 8 ||
> --
> 2.34.1
Yu, Gareth Feb. 21, 2025, 5:05 a.m. UTC | #2 
> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@intel.com>
> Sent: Friday, February 21, 2025 12:44 PM
> To: Yu, Gareth <gareth.yu@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: intel-xe@lists.freedesktop.org; Tseng, William <william.tseng@intel.com>;
> Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Heikkila, Juha-pekka <juha-
> pekka.heikkila@intel.com>
> Subject: RE: [PATCH] drm/i915/vdsc: Correct the conditions of DSC1.1 and
> DSC 1.2 for rc params calculation
> 
> 
> 
> > -----Original Message-----
> > From: Yu, Gareth <gareth.yu@intel.com>
> > Sent: Friday, February 21, 2025 9:44 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: intel-xe@lists.freedesktop.org; shawn.c.lee.intel.om; Tseng,
> > William <william.tseng@intel.com>; Nautiyal, Ankit K
> > <ankit.k.nautiyal@intel.com>; Yu, Gareth <gareth.yu@intel.com>;
> > Kandpal, Suraj <suraj.kandpal@intel.com>; Heikkila, Juha-pekka
> > <juha-pekka.heikkila@intel.com>
> > Subject: [PATCH] drm/i915/vdsc: Correct the conditions of DSC1.1 and
> > DSC 1.2 for rc params calculation
> >
> > From: Gareth Yu <gareth.yu@intel.com>
> >
> > The condition change is because Gen 14 begins to support DSC 1.2 and
> > need to check if the sink supports DSC1.2
> >
> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
> >
> > Cc: Suraj Kandpal <suraj.kandpal@intel.com>
> > Cc: Juha-Pekka Heikkil <juha-pekka.heikkila@intel.com>
> > Signed-off-by: Gareth Yu <gareth.yu@intel.com>
> 
> Why has the authorship and Signed-off-by changed for essentially the same
> patch.
> Also the commit message and versioning information has been removed any
> changes Needs to go as a new version to the previous patch.
This is a new patch to check both source and sink DSC version. The previous one (https://patchwork.freedesktop.org/patch/638403/?series=145136) is only to check sink condition.
> 
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_vdsc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > index b355c479eda3..555180e40b72 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > @@ -320,7 +320,7 @@ 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) >= 14) && (vdsc_cfg->dsc_version_minor
> >=
> > +2)) {
> 
> Please add a bspec reference number in the commit message where it says
> we started support of DSC 1.2 from MTL
BSPEC: 49259. Will add in the next version.
> 
> Regards,
> Suraj Kandpal
> 
> >  		calculate_rc_params(vdsc_cfg);
> >  	} else {
> >  		if ((compressed_bpp == 8 ||
> > --
> > 2.34.1
Kandpal, Suraj Feb. 21, 2025, 5:12 a.m. UTC | #3 
> -----Original Message-----
> From: Yu, Gareth <gareth.yu@intel.com>
> Sent: Friday, February 21, 2025 10:35 AM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: intel-xe@lists.freedesktop.org; Tseng, William <william.tseng@intel.com>;
> Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Heikkila, Juha-pekka <juha-
> pekka.heikkila@intel.com>
> Subject: RE: [PATCH] drm/i915/vdsc: Correct the conditions of DSC1.1 and DSC
> 1.2 for rc params calculation
> 
> 
> 
> > -----Original Message-----
> > From: Kandpal, Suraj <suraj.kandpal@intel.com>
> > Sent: Friday, February 21, 2025 12:44 PM
> > To: Yu, Gareth <gareth.yu@intel.com>; intel-gfx@lists.freedesktop.org
> > Cc: intel-xe@lists.freedesktop.org; Tseng, William
> > <william.tseng@intel.com>; Nautiyal, Ankit K
> > <ankit.k.nautiyal@intel.com>; Heikkila, Juha-pekka <juha-
> > pekka.heikkila@intel.com>
> > Subject: RE: [PATCH] drm/i915/vdsc: Correct the conditions of DSC1.1
> > and DSC 1.2 for rc params calculation
> >
> >
> >
> > > -----Original Message-----
> > > From: Yu, Gareth <gareth.yu@intel.com>
> > > Sent: Friday, February 21, 2025 9:44 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: intel-xe@lists.freedesktop.org; shawn.c.lee.intel.om; Tseng,
> > > William <william.tseng@intel.com>; Nautiyal, Ankit K
> > > <ankit.k.nautiyal@intel.com>; Yu, Gareth <gareth.yu@intel.com>;
> > > Kandpal, Suraj <suraj.kandpal@intel.com>; Heikkila, Juha-pekka
> > > <juha-pekka.heikkila@intel.com>
> > > Subject: [PATCH] drm/i915/vdsc: Correct the conditions of DSC1.1 and
> > > DSC 1.2 for rc params calculation
> > >
> > > From: Gareth Yu <gareth.yu@intel.com>
> > >
> > > The condition change is because Gen 14 begins to support DSC 1.2 and
> > > need to check if the sink supports DSC1.2
> > >
> > > Closes:
> > > https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
> > >
> > > Cc: Suraj Kandpal <suraj.kandpal@intel.com>
> > > Cc: Juha-Pekka Heikkil <juha-pekka.heikkila@intel.com>
> > > Signed-off-by: Gareth Yu <gareth.yu@intel.com>
> >
> > Why has the authorship and Signed-off-by changed for essentially the
> > same patch.
> > Also the commit message and versioning information has been removed
> > any changes Needs to go as a new version to the previous patch.
> This is a new patch to check both source and sink DSC version. The previous one
> (https://patchwork.freedesktop.org/patch/638403/?series=145136) is only to
> check sink condition.

That does not require a new patch series altogether a simple comment asking the previous
Author to add the DISPLAY_VER change should be the way to go. Add the DISPLAY_VERSION check
as a part of the older series.

Regards,
Suraj Kandpal

> >
> >
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_vdsc.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > index b355c479eda3..555180e40b72 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > @@ -320,7 +320,7 @@ 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) >= 14) && (vdsc_cfg->dsc_version_minor
> > >=
> > > +2)) {
> >
> > Please add a bspec reference number in the commit message where it
> > says we started support of DSC 1.2 from MTL
> BSPEC: 49259. Will add in the next version.
> >
> > Regards,
> > Suraj Kandpal
> >
> > >  		calculate_rc_params(vdsc_cfg);
> > >  	} else {
> > >  		if ((compressed_bpp == 8 ||
> > > --
> > > 2.34.1
Kandpal, Suraj Feb. 21, 2025, 5:26 a.m. UTC | #4 
> -----Original Message-----
> From: Yu, Gareth <gareth.yu@intel.com>
> Sent: Friday, February 21, 2025 10:35 AM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: intel-xe@lists.freedesktop.org; Tseng, William <william.tseng@intel.com>;
> Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Heikkila, Juha-pekka <juha-
> pekka.heikkila@intel.com>
> Subject: RE: [PATCH] drm/i915/vdsc: Correct the conditions of DSC1.1 and DSC
> 1.2 for rc params calculation
> 
> 
> 
> > -----Original Message-----
> > From: Kandpal, Suraj <suraj.kandpal@intel.com>
> > Sent: Friday, February 21, 2025 12:44 PM
> > To: Yu, Gareth <gareth.yu@intel.com>; intel-gfx@lists.freedesktop.org
> > Cc: intel-xe@lists.freedesktop.org; Tseng, William
> > <william.tseng@intel.com>; Nautiyal, Ankit K
> > <ankit.k.nautiyal@intel.com>; Heikkila, Juha-pekka <juha-
> > pekka.heikkila@intel.com>
> > Subject: RE: [PATCH] drm/i915/vdsc: Correct the conditions of DSC1.1
> > and DSC 1.2 for rc params calculation
> >
> >
> >
> > > -----Original Message-----
> > > From: Yu, Gareth <gareth.yu@intel.com>
> > > Sent: Friday, February 21, 2025 9:44 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: intel-xe@lists.freedesktop.org; shawn.c.lee.intel.om; Tseng,
> > > William <william.tseng@intel.com>; Nautiyal, Ankit K
> > > <ankit.k.nautiyal@intel.com>; Yu, Gareth <gareth.yu@intel.com>;
> > > Kandpal, Suraj <suraj.kandpal@intel.com>; Heikkila, Juha-pekka
> > > <juha-pekka.heikkila@intel.com>
> > > Subject: [PATCH] drm/i915/vdsc: Correct the conditions of DSC1.1 and
> > > DSC 1.2 for rc params calculation
> > >
> > > From: Gareth Yu <gareth.yu@intel.com>
> > >
> > > The condition change is because Gen 14 begins to support DSC 1.2 and
> > > need to check if the sink supports DSC1.2
> > >
> > > Closes:
> > > https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13719
> > >
> > > Cc: Suraj Kandpal <suraj.kandpal@intel.com>
> > > Cc: Juha-Pekka Heikkil <juha-pekka.heikkila@intel.com>
> > > Signed-off-by: Gareth Yu <gareth.yu@intel.com>
> >
> > Why has the authorship and Signed-off-by changed for essentially the
> > same patch.
> > Also the commit message and versioning information has been removed
> > any changes Needs to go as a new version to the previous patch.
> This is a new patch to check both source and sink DSC version. The previous one
> (https://patchwork.freedesktop.org/patch/638403/?series=145136) is only to
> check sink condition.

More things I forgot to mention the calculate RC Params also works for DSC 1.1
Check commit message of db514cac08fd4861a3b221bed5f21b441a1242c3
Also from what I gather we have had DSC1.2 support since ADLP so the whole condition in itself
Is not correct as I mentioned this needs more debug, follow up with logs asked in the issue.

Regards,
Suraj Kandpal

> >
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_vdsc.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > index b355c479eda3..555180e40b72 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > > @@ -320,7 +320,7 @@ 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) >= 14) && (vdsc_cfg->dsc_version_minor
> > >=
> > > +2)) {
> >
> > Please add a bspec reference number in the commit message where it
> > says we started support of DSC 1.2 from MTL
> BSPEC: 49259. Will add in the next version.
> >
> > Regards,
> > Suraj Kandpal
> >
> > >  		calculate_rc_params(vdsc_cfg);
> > >  	} else {
> > >  		if ((compressed_bpp == 8 ||
> > > --
> > > 2.34.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c
index b355c479eda3..555180e40b72 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -320,7 +320,7 @@  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) >= 14) && (vdsc_cfg->dsc_version_minor >= 2)) {
 		calculate_rc_params(vdsc_cfg);
 	} else {
 		if ((compressed_bpp == 8 ||