[PATCH 1/2] drm/i915: Do not enable FEC without DSC

[PATCH 1/2] drm/i915: Do not enable FEC without DSC

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we enable FEC even when DSC is no used. While that is
theoretically valid supposedly there isn't much of a benefit from
this. But more importantly we do not account for the FEC link
bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
So the code may think we have enough bandwidth when we in fact
do not.

Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.comk>
Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 326de12c3f44..bbf678561509 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
 	int pipe_bpp;
 	int ret;
 
+	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
+		intel_dp_supports_fec(intel_dp, pipe_config);
+
 	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
 		return -EINVAL;
 
@@ -2168,9 +2171,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
 		return -EINVAL;
 
-	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
-				  intel_dp_supports_fec(intel_dp, pipe_config);
-
 	ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state);
 	if (ret < 0)
 		return ret;
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: Clean up DSC vs. not bpp handling

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

No point in duplicating all this code when we can just
use a variable top hold the output bpp (the only thing
that differs between the two branches).

Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.comk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index bbf678561509..b26007a32318 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2126,7 +2126,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		to_intel_digital_connector_state(conn_state);
 	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
 					   DP_DPCD_QUIRK_CONSTANT_N);
-	int ret;
+	int ret, output_bpp;
 
 	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
 		pipe_config->has_pch_encoder = true;
@@ -2190,25 +2190,22 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
 	}
 
-	if (!pipe_config->dsc_params.compression_enable)
-		intel_link_compute_m_n(pipe_config->pipe_bpp,
-				       pipe_config->lane_count,
-				       adjusted_mode->crtc_clock,
-				       pipe_config->port_clock,
-				       &pipe_config->dp_m_n,
-				       constant_n);
+	if (pipe_config->dsc_params.compression_enable)
+		output_bpp = pipe_config->dsc_params.compressed_bpp;
 	else
-		intel_link_compute_m_n(pipe_config->dsc_params.compressed_bpp,
-				       pipe_config->lane_count,
-				       adjusted_mode->crtc_clock,
-				       pipe_config->port_clock,
-				       &pipe_config->dp_m_n,
-				       constant_n);
+		output_bpp = pipe_config->pipe_bpp;
+
+	intel_link_compute_m_n(output_bpp,
+			       pipe_config->lane_count,
+			       adjusted_mode->crtc_clock,
+			       pipe_config->port_clock,
+			       &pipe_config->dp_m_n,
+			       constant_n);
 
 	if (intel_connector->panel.downclock_mode != NULL &&
 		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
 			pipe_config->has_drrs = true;
-			intel_link_compute_m_n(pipe_config->pipe_bpp,
+			intel_link_compute_m_n(output_bpp,
 					       pipe_config->lane_count,
 					       intel_connector->panel.downclock_mode->clock,
 					       pipe_config->port_clock,
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Do not enable FEC without DSC

[Manasi Navare]

On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we enable FEC even when DSC is no used. While that is
> theoretically valid supposedly there isn't much of a benefit from
> this. But more importantly we do not account for the FEC link
> bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
> So the code may think we have enough bandwidth when we in fact
> do not.
> 
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.comk>
> Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 326de12c3f44..bbf678561509 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  	int pipe_bpp;
>  	int ret;
>  
> +	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> +		intel_dp_supports_fec(intel_dp, pipe_config);
> +

We could still not enable DSC after this point since it has more checks in this
function. Even though in that case we would fail the encoder config so wouldnt
matter if we have enabled FEC or not, but its less intutive.
IMHO, the ideal place to set the fec enable is in intel_dp_compute_link_config()
after the all to dsc_compute_config and set it only if pipe_config->dsc_params.compression_enable

Manasi

>  	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
>  		return -EINVAL;
>  
> @@ -2168,9 +2171,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>  		return -EINVAL;
>  
> -	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> -				  intel_dp_supports_fec(intel_dp, pipe_config);
> -
>  	ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state);
>  	if (ret < 0)
>  		return ret;
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Clean up DSC vs. not bpp handling

[Manasi Navare]

On Tue, Mar 26, 2019 at 04:49:03PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> No point in duplicating all this code when we can just
> use a variable top hold the output bpp (the only thing
> that differs between the two branches).
> 
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.comk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

This clean up looks good, thank you for catching this and cleaning it up.

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index bbf678561509..b26007a32318 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2126,7 +2126,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  		to_intel_digital_connector_state(conn_state);
>  	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
>  					   DP_DPCD_QUIRK_CONSTANT_N);
> -	int ret;
> +	int ret, output_bpp;
>  
>  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
>  		pipe_config->has_pch_encoder = true;
> @@ -2190,25 +2190,22 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
>  	}
>  
> -	if (!pipe_config->dsc_params.compression_enable)
> -		intel_link_compute_m_n(pipe_config->pipe_bpp,
> -				       pipe_config->lane_count,
> -				       adjusted_mode->crtc_clock,
> -				       pipe_config->port_clock,
> -				       &pipe_config->dp_m_n,
> -				       constant_n);
> +	if (pipe_config->dsc_params.compression_enable)
> +		output_bpp = pipe_config->dsc_params.compressed_bpp;
>  	else
> -		intel_link_compute_m_n(pipe_config->dsc_params.compressed_bpp,
> -				       pipe_config->lane_count,
> -				       adjusted_mode->crtc_clock,
> -				       pipe_config->port_clock,
> -				       &pipe_config->dp_m_n,
> -				       constant_n);
> +		output_bpp = pipe_config->pipe_bpp;
> +
> +	intel_link_compute_m_n(output_bpp,
> +			       pipe_config->lane_count,
> +			       adjusted_mode->crtc_clock,
> +			       pipe_config->port_clock,
> +			       &pipe_config->dp_m_n,
> +			       constant_n);
>  
>  	if (intel_connector->panel.downclock_mode != NULL &&
>  		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
>  			pipe_config->has_drrs = true;
> -			intel_link_compute_m_n(pipe_config->pipe_bpp,
> +			intel_link_compute_m_n(output_bpp,
>  					       pipe_config->lane_count,
>  					       intel_connector->panel.downclock_mode->clock,
>  					       pipe_config->port_clock,
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Do not enable FEC without DSC

[Ville Syrjälä]

On Tue, Mar 26, 2019 at 09:00:27AM -0700, Manasi Navare wrote:
> On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently we enable FEC even when DSC is no used. While that is
> > theoretically valid supposedly there isn't much of a benefit from
> > this. But more importantly we do not account for the FEC link
> > bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
> > So the code may think we have enough bandwidth when we in fact
> > do not.
> > 
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.comk>
> > Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 326de12c3f44..bbf678561509 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> >  	int pipe_bpp;
> >  	int ret;
> >  
> > +	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > +		intel_dp_supports_fec(intel_dp, pipe_config);
> > +
> 
> We could still not enable DSC after this point since it has more checks in this
> function. Even though in that case we would fail the encoder config so wouldnt
> matter if we have enabled FEC or not, but its less intutive.
> IMHO, the ideal place to set the fec enable is in intel_dp_compute_link_config()
> after the all to dsc_compute_config and set it only if pipe_config->dsc_params.compression_enable

That would require changing intel_dp_supports_dsc() which I decided
wasn't worth the hassle.

> 
> Manasi
> 
> >  	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> >  		return -EINVAL;
> >  
> > @@ -2168,9 +2171,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> >  		return -EINVAL;
> >  
> > -	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > -				  intel_dp_supports_fec(intel_dp, pipe_config);
> > -
> >  	ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state);
> >  	if (ret < 0)
> >  		return ret;
> > -- 
> > 2.19.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Do not enable FEC without DSC

[Manasi Navare]

On Tue, Mar 26, 2019 at 06:16:57PM +0200, Ville Syrjälä wrote:
> On Tue, Mar 26, 2019 at 09:00:27AM -0700, Manasi Navare wrote:
> > On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Currently we enable FEC even when DSC is no used. While that is
> > > theoretically valid supposedly there isn't much of a benefit from
> > > this. But more importantly we do not account for the FEC link
> > > bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
> > > So the code may think we have enough bandwidth when we in fact
> > > do not.
> > > 
> > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > Cc: Manasi Navare <manasi.d.navare@intel.comk>
> > > Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 326de12c3f44..bbf678561509 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> > >  	int pipe_bpp;
> > >  	int ret;
> > >  
> > > +	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > > +		intel_dp_supports_fec(intel_dp, pipe_config);
> > > +
> > 
> > We could still not enable DSC after this point since it has more checks in this
> > function. Even though in that case we would fail the encoder config so wouldnt
> > matter if we have enabled FEC or not, but its less intutive.
> > IMHO, the ideal place to set the fec enable is in intel_dp_compute_link_config()
> > after the all to dsc_compute_config and set it only if pipe_config->dsc_params.compression_enable
> 
> That would require changing intel_dp_supports_dsc() which I decided
> wasn't worth the hassle.

Hmm, yea because intel_dp_supports_dsc depends on fec_enable.
In that case we would need to change that to use intel_dp_supports_fec(). TBH that makes more sense since
we should set dp_supports based on HW FEC capability and then set the crtc_state->fec_enable based
on crtc_state->dsc_params.compression_enable. But since it doesnt change the functionality, I am ok
either ways.

Manasi

> 
> > 
> > Manasi
> > 
> > >  	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> > >  		return -EINVAL;
> > >  
> > > @@ -2168,9 +2171,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> > >  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> > >  		return -EINVAL;
> > >  
> > > -	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > > -				  intel_dp_supports_fec(intel_dp, pipe_config);
> > > -
> > >  	ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state);
> > >  	if (ret < 0)
> > >  		return ret;
> > > -- 
> > > 2.19.2
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Do not enable FEC without DSC

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Do not enable FEC without DSC
URL   : https://patchwork.freedesktop.org/series/58588/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5818 -> Patchwork_12602
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58588/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12602 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      PASS -> FAIL [fdo#108511]

  * igt@i915_selftest@live_uncore:
    - fi-ivb-3770:        PASS -> DMESG-FAIL [fdo#110210]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103167]

  
#### Possible fixes ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-hsw-4770:        SKIP [fdo#109271] -> PASS +2

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - fi-icl-u3:          INCOMPLETE [fdo#108569] -> DMESG-FAIL [fdo#108569]

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210


Participating hosts (45 -> 38)
------------------------------

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-gdg-551 


Build changes
-------------

    * Linux: CI_DRM_5818 -> Patchwork_12602

  CI_DRM_5818: de0e80842f3d103996e99cfe27f999690c2ee06e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4905: a350b9f9f606296b1599c3617c8530a8985709e2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12602: d4765b6682ad9caeb34c3bab5eb6e0d3eeb4e483 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d4765b6682ad drm/i915: Clean up DSC vs. not bpp handling
ebed6de93f4d drm/i915: Do not enable FEC without DSC

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12602/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915: Do not enable FEC without DSC

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Do not enable FEC without DSC
URL   : https://patchwork.freedesktop.org/series/58588/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5818_full -> Patchwork_12602_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with Patchwork_12602_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12602_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12602_full:

### IGT changes ###

#### Warnings ####

  * igt@kms_setmode@basic:
    - shard-iclb:         FAIL [fdo#99912] -> INCOMPLETE

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@gem_exec_big@single}:
    - shard-kbl:          PASS -> FAIL

  
Known issues
------------

  Here are the changes found in Patchwork_12602_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_parallel@bsd2-fds:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +6

  * igt@gem_mocs_settings@mocs-rc6-dirty-render:
    - shard-iclb:         NOTRUN -> SKIP [fdo#110206]

  * igt@gem_ppgtt@blt-vs-render-ctxn:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#109801]

  * igt@gem_pread@pagefault-pread:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109277]

  * igt@gem_workarounds@suspend-resume-context:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773] +1

  * igt@i915_pm_rpm@gem-execbuf-stress-pc8:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109506]

  * igt@i915_pm_rpm@modeset-lpsp-stress-no-wait:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +22

  * igt@i915_pm_rpm@modeset-non-lpsp-stress:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109308]

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107807]

  * igt@i915_pm_rps@min-max-config-loaded:
    - shard-iclb:         NOTRUN -> FAIL [fdo#108059]

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#110222] +4

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-f:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#110222]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-d:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +4

  * igt@kms_chamelium@hdmi-cmp-planes-random:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284] +2

  * igt@kms_cursor_legacy@2x-cursor-vs-flip-legacy:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274] +3

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103355] +1

  * igt@kms_cursor_legacy@cursor-vs-flip-legacy:
    - shard-iclb:         PASS -> FAIL [fdo#103355]

  * igt@kms_fbcon_fbt@psr:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103833]

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          NOTRUN -> FAIL [fdo#103833]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          NOTRUN -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          PASS -> FAIL [fdo#102887] / [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-skl:          PASS -> FAIL [fdo#103167]
    - shard-iclb:         PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-skl:          PASS -> FAIL [fdo#105682]
    - shard-kbl:          PASS -> FAIL [fdo#105682] +1

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-cpu:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +71

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +10

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
    - shard-iclb:         NOTRUN -> FAIL [fdo#109247] +3

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +19

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-cpu:
    - shard-iclb:         PASS -> FAIL [fdo#105682] / [fdo#109247]

  * igt@kms_panel_fitting@legacy:
    - shard-skl:          NOTRUN -> FAIL [fdo#105456]

  * igt@kms_pipe_b_c_ivb@from-pipe-c-to-b-with-3-lanes:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109289] +2

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-iclb:         PASS -> FAIL [fdo#103375]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-f:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +17

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-kbl:          PASS -> FAIL [fdo#108145]
    - shard-skl:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108590]

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +9

  * igt@kms_plane_scaling@pipe-c-scaler-with-pixel-format:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         PASS -> SKIP [fdo#109642]

  * igt@kms_psr@primary_mmap_cpu:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215] +2

  * igt@kms_psr@primary_render:
    - shard-iclb:         NOTRUN -> FAIL [fdo#107383] / [fdo#110215]

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +1

  * igt@kms_setmode@basic:
    - shard-snb:          NOTRUN -> FAIL [fdo#99912]

  * igt@kms_vblank@pipe-b-ts-continuation-modeset-hang:
    - shard-apl:          PASS -> FAIL [fdo#104894] +1

  * igt@perf_pmu@busy-accuracy-50-vcs1:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +170

  * igt@prime_nv_api@nv_self_import_to_different_fd:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109291] +2

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@vecs0-s3:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS

  * {igt@gem_exec_big@single}:
    - shard-glk:          FAIL -> PASS

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         DMESG-WARN [fdo#108686] -> PASS

  * igt@i915_pm_rpm@modeset-lpsp:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS +1

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          FAIL [fdo#105363] -> PASS

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +17

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +6

  * {igt@kms_plane@pixel-format-pipe-a-planes-source-clamping}:
    - shard-glk:          SKIP [fdo#109271] -> PASS

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +2

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          FAIL [fdo#109016] -> PASS

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-iclb:         FAIL [fdo#104894] -> PASS

  
#### Warnings ####

  * igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait:
    - shard-skl:          INCOMPLETE [fdo#107807] -> SKIP [fdo#109271]

  * igt@i915_selftest@live_contexts:
    - shard-iclb:         INCOMPLETE [fdo#108569] -> DMESG-FAIL [fdo#108569]

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> FAIL [fdo#109016]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103833]: https://bugs.freedesktop.org/show_bug.cgi?id=103833
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108059]: https://bugs.freedesktop.org/show_bug.cgi?id=108059
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
  [fdo#110206]: https://bugs.freedesktop.org/show_bug.cgi?id=110206
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


Build changes
-------------

    * Linux: CI_DRM_5818 -> Patchwork_12602

  CI_DRM_5818: de0e80842f3d103996e99cfe27f999690c2ee06e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4905: a350b9f9f606296b1599c3617c8530a8985709e2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12602: d4765b6682ad9caeb34c3bab5eb6e0d3eeb4e483 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12602/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Do not enable FEC without DSC

[Ville Syrjälä]

On Tue, Mar 26, 2019 at 11:10:44AM -0700, Manasi Navare wrote:
> On Tue, Mar 26, 2019 at 06:16:57PM +0200, Ville Syrjälä wrote:
> > On Tue, Mar 26, 2019 at 09:00:27AM -0700, Manasi Navare wrote:
> > > On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Currently we enable FEC even when DSC is no used. While that is
> > > > theoretically valid supposedly there isn't much of a benefit from
> > > > this. But more importantly we do not account for the FEC link
> > > > bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
> > > > So the code may think we have enough bandwidth when we in fact
> > > > do not.
> > > > 
> > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > > Cc: Manasi Navare <manasi.d.navare@intel.comk>
> > > > Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 326de12c3f44..bbf678561509 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> > > >  	int pipe_bpp;
> > > >  	int ret;
> > > >  
> > > > +	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > > > +		intel_dp_supports_fec(intel_dp, pipe_config);
> > > > +
> > > 
> > > We could still not enable DSC after this point since it has more checks in this
> > > function. Even though in that case we would fail the encoder config so wouldnt
> > > matter if we have enabled FEC or not, but its less intutive.
> > > IMHO, the ideal place to set the fec enable is in intel_dp_compute_link_config()
> > > after the all to dsc_compute_config and set it only if pipe_config->dsc_params.compression_enable
> > 
> > That would require changing intel_dp_supports_dsc() which I decided
> > wasn't worth the hassle.
> 
> Hmm, yea because intel_dp_supports_dsc depends on fec_enable.
> In that case we would need to change that to use intel_dp_supports_fec(). TBH that makes more sense since
> we should set dp_supports based on HW FEC capability and then set the crtc_state->fec_enable based
> on crtc_state->dsc_params.compression_enable. But since it doesnt change the functionality, I am ok
> either ways.

So r-b?

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Clean up DSC vs. not bpp handling

[Ville Syrjälä]

On Tue, Mar 26, 2019 at 09:02:36AM -0700, Manasi Navare wrote:
> On Tue, Mar 26, 2019 at 04:49:03PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > No point in duplicating all this code when we can just
> > use a variable top hold the output bpp (the only thing
> > that differs between the two branches).
> > 
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.comk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> This clean up looks good, thank you for catching this and cleaning it up.
> 
> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Patch 2 pushed. Thanks for the review. Patch 1 could still use a rubber
stamp...

> 
> Manasi
> 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 27 ++++++++++++---------------
> >  1 file changed, 12 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index bbf678561509..b26007a32318 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2126,7 +2126,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >  		to_intel_digital_connector_state(conn_state);
> >  	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
> >  					   DP_DPCD_QUIRK_CONSTANT_N);
> > -	int ret;
> > +	int ret, output_bpp;
> >  
> >  	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
> >  		pipe_config->has_pch_encoder = true;
> > @@ -2190,25 +2190,22 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >  			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
> >  	}
> >  
> > -	if (!pipe_config->dsc_params.compression_enable)
> > -		intel_link_compute_m_n(pipe_config->pipe_bpp,
> > -				       pipe_config->lane_count,
> > -				       adjusted_mode->crtc_clock,
> > -				       pipe_config->port_clock,
> > -				       &pipe_config->dp_m_n,
> > -				       constant_n);
> > +	if (pipe_config->dsc_params.compression_enable)
> > +		output_bpp = pipe_config->dsc_params.compressed_bpp;
> >  	else
> > -		intel_link_compute_m_n(pipe_config->dsc_params.compressed_bpp,
> > -				       pipe_config->lane_count,
> > -				       adjusted_mode->crtc_clock,
> > -				       pipe_config->port_clock,
> > -				       &pipe_config->dp_m_n,
> > -				       constant_n);
> > +		output_bpp = pipe_config->pipe_bpp;
> > +
> > +	intel_link_compute_m_n(output_bpp,
> > +			       pipe_config->lane_count,
> > +			       adjusted_mode->crtc_clock,
> > +			       pipe_config->port_clock,
> > +			       &pipe_config->dp_m_n,
> > +			       constant_n);
> >  
> >  	if (intel_connector->panel.downclock_mode != NULL &&
> >  		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
> >  			pipe_config->has_drrs = true;
> > -			intel_link_compute_m_n(pipe_config->pipe_bpp,
> > +			intel_link_compute_m_n(output_bpp,
> >  					       pipe_config->lane_count,
> >  					       intel_connector->panel.downclock_mode->clock,
> >  					       pipe_config->port_clock,
> > -- 
> > 2.19.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Do not enable FEC without DSC

[Manasi Navare]

On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we enable FEC even when DSC is no used. While that is
> theoretically valid supposedly there isn't much of a benefit from
> this. But more importantly we do not account for the FEC link
> bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
> So the code may think we have enough bandwidth when we in fact
> do not.
> 
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.comk>

There is a typo in the email address:
manasi.d.navare@intel.com

> Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Makes sense to me

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 326de12c3f44..bbf678561509 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
>  	int pipe_bpp;
>  	int ret;
>  
> +	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> +		intel_dp_supports_fec(intel_dp, pipe_config);
> +
>  	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
>  		return -EINVAL;
>  
> @@ -2168,9 +2171,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>  		return -EINVAL;
>  
> -	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> -				  intel_dp_supports_fec(intel_dp, pipe_config);
> -
>  	ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state);
>  	if (ret < 0)
>  		return ret;
> -- 
> 2.19.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Do not enable FEC without DSC

[Ville Syrjälä]

On Thu, Apr 11, 2019 at 12:11:42PM -0700, Manasi Navare wrote:
> On Tue, Mar 26, 2019 at 04:49:02PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently we enable FEC even when DSC is no used. While that is
> > theoretically valid supposedly there isn't much of a benefit from
> > this. But more importantly we do not account for the FEC link
> > bandwidth overhead (2.4%) in the non-DSC link bandwidth computations.
> > So the code may think we have enough bandwidth when we in fact
> > do not.
> > 
> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.comk>
> 
> There is a typo in the email address:
> manasi.d.navare@intel.com

Thanks for the reminder. I already forgot about this and was about to
push without fixing it. Sadly it seems I had copy pasted it from the
same place to both of these patches, so one got pushed with the bad
address.

Hmm. Looks like this escaped into 5.0 already. Added cc:stable
and pushed to dinq. Thanks for the review.

> 
> > Fixes: 240999cf339f ("i915/dp/fec: Add fec_enable to the crtc state.")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Makes sense to me
> 
> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> 
> Manasi
> 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 326de12c3f44..bbf678561509 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1925,6 +1925,9 @@ static int intel_dp_dsc_compute_config(struct intel_dp *intel_dp,
> >  	int pipe_bpp;
> >  	int ret;
> >  
> > +	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > +		intel_dp_supports_fec(intel_dp, pipe_config);
> > +
> >  	if (!intel_dp_supports_dsc(intel_dp, pipe_config))
> >  		return -EINVAL;
> >  
> > @@ -2168,9 +2171,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> >  		return -EINVAL;
> >  
> > -	pipe_config->fec_enable = !intel_dp_is_edp(intel_dp) &&
> > -				  intel_dp_supports_fec(intel_dp, pipe_config);
> > -
> >  	ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state);
> >  	if (ret < 0)
> >  		return ret;
> > -- 
> > 2.19.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx